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

don't localize imported values in selectors #13

Merged
merged 3 commits into from
May 7, 2019

Conversation

jquense
Copy link
Contributor

@jquense jquense commented Mar 7, 2019

This PR adds support for using imported values as selectors. Previously if an imported value (like a class name) was used in a selector it would get replaces correctly, but then would be made :local() which ends up resulting in the classname getting hashed again later and removing the reference to the original value.

This prevents that by treating an instance of a imported key in a selector as if it was global so that it's not localized. You can also avoid this behavior by explicitly making the selector :local()

@jquense
Copy link
Contributor Author

jquense commented Mar 7, 2019

@evilebottnawi this is, I think, a better alternative to css-modules/postcss-modules-scope#4 and needed before webpack-contrib/css-loader#894

@codecov-io
Copy link

codecov-io commented Mar 7, 2019

Codecov Report

Merging #13 into master will decrease coverage by 0.32%.
The diff coverage is 95.83%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #13      +/-   ##
==========================================
- Coverage   99.63%   99.31%   -0.33%     
==========================================
  Files           2        2              
  Lines         273      291      +18     
  Branches       86       91       +5     
==========================================
+ Hits          272      289      +17     
- Misses          1        2       +1
Impacted Files Coverage Δ
test.js 100% <ø> (ø) ⬆️
index.js 99.23% <95.83%> (-0.36%) ⬇️

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 ba6229a...d1c2111. Read the comment docs.

@alexander-akait
Copy link
Member

Good job! Can you describe more verbosely what PR do for other developers. Also looks we have regression, can you look on this webpack-contrib/css-loader#906?

index.js Outdated Show resolved Hide resolved
@alexander-akait
Copy link
Member

hm, looks like we should release this as major version, right?

@jquense
Copy link
Contributor Author

jquense commented Mar 11, 2019

I would probably bump the major to be safe yeah.

@jquense
Copy link
Contributor Author

jquense commented Apr 1, 2019

checkin: I am done here, unless there is something you'd like me to change :)

@alexander-akait
Copy link
Member

All god, thanks for PR

@alexander-akait alexander-akait merged commit ff9580a into css-modules:master May 7, 2019
@jquense jquense deleted the value-in-selectors branch May 7, 2019 13:04
@jquense
Copy link
Contributor Author

jquense commented May 7, 2019

awesome thanks!

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