-
Notifications
You must be signed in to change notification settings - Fork 192
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
Add granular CSS Modules options #739
Add granular CSS Modules options #739
Conversation
napi/src/lib.rs
Outdated
@@ -713,6 +716,9 @@ fn compile<'i>( | |||
Default::default() | |||
}, | |||
dashed_idents: c.dashed_idents.unwrap_or_default(), | |||
animation: c.animation.unwrap_or_default(), | |||
grid: c.grid.unwrap_or_default(), | |||
custom_idents: c.custom_idents.unwrap_or_default(), |
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.
These should probably default to true like below.
src/properties/animation.rs
Outdated
@@ -58,17 +58,22 @@ impl<'i> ToCss for AnimationName<'i> { | |||
where | |||
W: std::fmt::Write, | |||
{ | |||
let css_module_animation_enabled = | |||
dest.css_module.as_mut().map_or(false, |css_module| css_module.config.animation); |
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.
dest.css_module.as_mut().map_or(false, |css_module| css_module.config.animation); | |
dest.css_module.as_ref().map_or(false, |css_module| css_module.config.animation); |
src/values/ident.rs
Outdated
.as_mut() | ||
.map_or(false, |css_module| css_module.config.custom_idents); | ||
|
||
dest.write_dashed_ident(&self.0, true, css_module_custom_idents_enabled) |
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.
write_dashed_ident
already checks the dashed_idents
option. I'm not sure that the custom_idents
option also needs to affect dashed idents as well?
src/properties/animation.rs
Outdated
@@ -58,17 +58,22 @@ impl<'i> ToCss for AnimationName<'i> { | |||
where | |||
W: std::fmt::Write, | |||
{ | |||
let css_module_animation_enabled = | |||
dest.css_module.as_mut().map_or(false, |css_module| css_module.config.animation); | |||
|
|||
match self { | |||
AnimationName::None => dest.write_str("none"), | |||
AnimationName::Ident(s) => { | |||
if let Some(css_module) = &mut dest.css_module { | |||
css_module.reference(&s.0, dest.loc.source_index) |
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.
need the check here too
e38e6bd
to
39f4311
Compare
39f4311
to
e5d9ca1
Compare
Reverts #8060 parcel-bundler/lightningcss#739 is now applied so we don't need this lint anymore
Reverts vercel/turborepo#8060 parcel-bundler/lightningcss#739 is now applied so we don't need this lint anymore
Reverts vercel/turborepo#8060 parcel-bundler/lightningcss#739 is now applied so we don't need this lint anymore
Reverts vercel/turborepo#8060 parcel-bundler/lightningcss#739 is now applied so we don't need this lint anymore
Reverts vercel/turborepo#8060 parcel-bundler/lightningcss#739 is now applied so we don't need this lint anymore
Change in this PR
Adds granular options to switch on/off CSS module scoping for CSS features that are prefixed by default in Lightning CSS currently. For example animation, grid, and custom identifiers.
The reason this is useful is that for example Webpack's CSS modules implementation does not do prefixing of
grid
. If you're migrating to Lightning CSS, like we're doing in Next.js / Turbopack it's helpful to disable the grid prefixing feature to ensure people have a smooth experience migrating.Last test failure
The PR is mostly done, just one more case with
animation: false
where it somehow usesCustomIdent
which in turn means it still prefixes.PR feedback
I'm not proficient in writing Rust currently, using working on these smaller changes in Turbopack and other deps of Turbopack to learn as well, let me know if anything should be changed 👍
Improved
assert_eq
failure reports in testsWhile investigating mismatches it was quite hard to read the
assert_eq
for these outputs when they mismatch. Addedpretty_assertions
to add much better diffs for these cases:Before:
After: