Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow modules.getLocalIdent to return a falsy value #963

Merged

Conversation

ziir
Copy link
Contributor

@ziir ziir commented Jun 28, 2019

This PR contains a:

  • bugfix
  • new feature
  • code refactor
  • test update
  • typo fix
  • metadata update

Motivation / Use-Case

With the suggested changes, modules.getLocalIdent would be allowed to return a falsy value, and in such case, the default getLocalIdent would be used instead.

I am setting up a new project with webpack, I wish to apply the same chain of loaders on different kinds of .scss files:

  • main.scss specified as an entry in my configuration, meant to be used as global stylesheet
  • App.scss SCSS modules which are imported from their matching component such as App.js

This setup works very well for me, except that main.scss classnames necessarily go through css-loader with modules enabled and using localIdentName.

See the following example:

{
  test: /\.scss$/,
  loader: [
    MiniCSSExtractPlugin.loader,
    {
      loader: 'css-loader',
      options: {
        importLoaders: 1,
        modules: {
          localIdentName: '[name]-[local]__[hash:base64:5]',
        },
      },
    },
    {
      loader: 'sass-loader',
    },
  ],
},

While browsing issues, looking for a way to not use localIdentName for my main.scss file, I stumbled upon #862.
getLocalIdent: () => foo && 'bar' seemed to be a great fit for my use-case.
The behavior introduced in #865 wouldn't have worked for me though, here's what I did instinctively:

modules: {
  localIdentName: '[name]-[local]__[hash:base64:5]',
  getLocalIdent: (context, localIdentName, localName) =>
    context.context.includes('shared/assets/styles') && localName,
},

Note: my main.scss file resides in shared/assets/styles.
This way, my SCSS modules will use localIdentName and main.scss will use localName.

Breaking Changes

None

Additional Info

Thank you for this great package :)
I'm willing to make the final changes if this looks reasonable, thank you in advance for any response.

@jsf-clabot
Copy link

jsf-clabot commented Jun 28, 2019

CLA assistant check
All committers have signed the CLA.

@alexander-akait
Copy link
Member

Please accept CLA

@alexander-akait
Copy link
Member

@ziir
Copy link
Contributor Author

ziir commented Jun 28, 2019

@evilebottnawi Thanks, signed the CLA.

I think I'm missing something, localIdentName remains a string and I cannot update getLocalIdent function return type?

I'm also not sure why some buils are failing.

@alexander-akait
Copy link
Member

i was wrong, need update snapshots npm run test:only -- -u

@ziir
Copy link
Contributor Author

ziir commented Jun 28, 2019

I already have updated the snapshots:
bca9163#diff-3cb22b4647b22161352a612896f9eaa8

@alexander-akait
Copy link
Member

@ziir update webpack to latest version in package.json and package-lock.json and run updating again

@ziir
Copy link
Contributor Author

ziir commented Jun 28, 2019

@evilebottnawi thnx, all checks are green now :)

@alexander-akait alexander-akait merged commit 9c3571c into webpack-contrib:master Jun 28, 2019
@alexander-akait
Copy link
Member

Thanks, release will be on next week (Monday i think)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants