-
-
Notifications
You must be signed in to change notification settings - Fork 606
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: support imported classes in selectors (postcss-icss
)
#562
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@apexskier Let's maybe wait until @TrySound can take a look at it, since he wrote the new implemenation and can comment on the possibilty to land this before 'new-loader' becomes a thing. Otherwise we better focus on landing that 😛
"postcss-modules-local-by-default": "^1.0.1", | ||
"postcss-modules-scope": "^1.0.0", | ||
"postcss-modules-values": "^1.1.0", | ||
"postcss-icss-composes": "^2.0.3", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
postcss-icss-keyframes
is missing, that was splitted into it's own plugin :) for @keyframes
support
Maybe postcss-icss-import
(@import
) && postcss-icss-url
(url()
) aswell
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
postcss-icss-url
seems to break a bunch of stuff for me on first testing. Might not end up being worthwhile with the new-loader
branch being in progress. I'd still like to get css-modules/postcss-icss-selectors#119 merged, since I think it'll be required even with new-loader
and the reorganization of icss loaders.
}), | ||
icssComposes, | ||
parserPlugin(parserOptions) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the contract logic changes with renaming the various postcss-modules-*
to postcss-icss-*
my gut feeling tells me, that this will require a few changes to that internal plugin (parserPlugin
) aswell
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I've covered a bit of that above, but yeah, I'm not sure about everything.
postcss-icss
)
As it is likely for this to not work out, I'm closing the PR for now. Thx :) |
It's been a year since this was closed, and it's entirely unclear what is supposed to replace it. What am I supposed to use when I need this functionality (that is part of the spec)? |
@joepie91 in near future we release |
@evilebottnawi If I need support for this right now, is there already a release candidate or Git branch or something that I can use for the time being? Or does the code for supporting it not exist at all yet? (I'd be happy to test it and provide feedback.) |
Still work in progress, opening for visibility and guidance. Currently works locally but due to dependency updates tests don't pass.
What kind of change does this PR introduce?
Adds support for importing css modules in selectors. I guess it's a new feature, since this wasn't supported, but I'd consider it more of a bug fix, since the feature is part of the spec.
Did you add tests for your changes?
Summary
This add support for importing css selectors from other locally scoped css files. This feature has been requested a few places and, in my opinion, allows css modules to use one of the core features that makes css great: being able to style components contextually.
A quick example of usage (using bare icss syntax):
Previously, this would have been compiled to something like:
Note that the
.btn
class has been scoped locally to the grouped button styles.Now, this compiles to:
Does this PR introduce a breaking change?
Possibly, this updates dependencies, and the base
icss-utils
repo has had a major version bump. I've got to investigate why tests are failing to know for sure. I'll update if so.The main changes are based on css-modules/icss-utils@19ffee9
Also, thanks to @michael-ciniawsky for some context here - see #561 (comment)