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: export default getLocalIdent #1046

Closed
wants to merge 1 commit into from
Closed

feat: export default getLocalIdent #1046

wants to merge 1 commit into from

Conversation

niki-timofe
Copy link

@niki-timofe niki-timofe commented Jan 22, 2020

This PR contains a:

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

Motivation / Use-Case

For example, use default getLocalIdent, but override it's behavior for specific files:

const getLocalIdent = require('css-loader/dist/utils').getLocalIdent;

...

            {
                loader: 'css-loader',
                options: {
                    modules: {
                        localIdentName: '[local]--[hash:base64:5]',
                        getLocalIdent: (context, localIdentName, localName, options) => getLocalIdent(
                            context,
                            context.resourcePath.endsWith('.modules.scss')
                                ? localIdentName
                                : '[local]',
                            localName,
                            options,
                        ),
                    },
                },
            }

@jsf-clabot
Copy link

jsf-clabot commented Jan 22, 2020

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented Jan 22, 2020

Codecov Report

Merging #1046 into master will not change coverage by %.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1046   +/-   ##
=======================================
  Coverage   98.14%   98.14%           
=======================================
  Files          10       10           
  Lines         484      484           
  Branches      150      150           
=======================================
  Hits          475      475           
  Misses          8        8           
  Partials        1        1           
Impacted Files Coverage Δ
src/utils.js 98.96% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 229d36a...19edda3. Read the comment docs.

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure we should do that, because we change algorithm often, so it will be problem with released and require for us do major release on each change

@niki-timofe
Copy link
Author

I understand. May be adding option for filtering files will be better solution?

@alexander-akait
Copy link
Member

Which files?

@niki-timofe
Copy link
Author

niki-timofe commented Jan 23, 2020

As in example, I've bunch of legacy files, which not supposed to use css-modules.

UPD:
Ok. Now I realized that i can just create alternative rule for them in webpack 🙃

@alexander-akait
Copy link
Member

Other good solution what we can implement #994

@alexander-akait
Copy link
Member

We can implement that right now (test option should be false in current version, but we can set it to true in next release)

@niki-timofe
Copy link
Author

Oh, it's cool. I'll try to help implement that.

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.

3 participants