Skip to content

css: parse composes as CSS modules syntax only in a CSS module #45961

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.

Sign up for GitHub

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

Closed
robobun wants to merge 4 commits into from
Closed

css: parse composes as CSS modules syntax only in a CSS module #45961

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
543b3e4
css: parse `composes` as CSS modules syntax only in a CSS module
robobun Oct 18, 2026
c9cfac8
css: remove the printer checks that the `composes` guard makes always…
robobun Oct 18, 2026
757eba4
css: remove the parser flag test that a `Composes` value now implies
robobun Oct 18, 2026
2ab9346
css: shorten the comment on the `handle_composes` call
robobun Oct 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 25 additions & 27 deletions src/css/declaration.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,33 +347,31 @@ where
.is_ok();
input.expect_exhausted()?;

if input.flags.css_modules() {
if let css::Property::Composes(composes) = &mut property {
match composes_ctx.composes_state() {
css::ComposesState::DisallowEntirely => {}
css::ComposesState::Allow(_) => {
composes_ctx.record_composes(composes);
}
css::ComposesState::DisallowNested(info) => {
options.warn_fmt(
format_args!("\"composes\" is not allowed inside nested selectors"),
info.line,
info.column,
);
}
css::ComposesState::DisallowNotSingleClass(info) => {
options.warn_fmt_with_notes(
format_args!("\"composes\" only works inside single class selectors"),
source_location.line,
source_location.column,
Box::new([bun_ast::Data {
text: b"The parent selector is not a single class selector because of the syntax here:"
.as_slice()
.into(),
location: Some(info.to_logger_location(options.filename)),
}]),
);
}
if let css::Property::Composes(composes) = &mut property {
match composes_ctx.composes_state() {
css::ComposesState::DisallowEntirely => {}
css::ComposesState::Allow(_) => {
composes_ctx.record_composes(composes);
}
css::ComposesState::DisallowNested(info) => {
options.warn_fmt(
format_args!("\"composes\" is not allowed inside nested selectors"),
info.line,
info.column,
);
}
css::ComposesState::DisallowNotSingleClass(info) => {
options.warn_fmt_with_notes(
format_args!("\"composes\" only works inside single class selectors"),
source_location.line,
source_location.column,
Box::new([bun_ast::Data {
text: b"The parent selector is not a single class selector because of the syntax here:"
.as_slice()
.into(),
location: Some(info.to_logger_location(options.filename)),
}]),
);
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/css/properties/properties_generated.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3684,10 +3684,12 @@ impl Property {
return Ok(Property::Direction(c));
}
}
PropertyId::Composes => {
// Not `options.css_modules`: it is a default in @keyframes and @container style().
PropertyId::Composes if input.flags.css_modules() => {
Comment thread
claude[bot] marked this conversation as resolved.
Resolved
Comment thread
robobun marked this conversation as resolved.
Resolved
Comment thread
robobun marked this conversation as resolved.
Resolved
return css::generic::parse_with_options::<css_modules::Composes>(input, options)
.map(Property::Composes);
}
PropertyId::Composes => {}
PropertyId::MaskImage(pre) => {
if let Some(c) =
parse_value::<SmallList<css::css_values::image::Image, 1>>(input, options)
Expand Down
45 changes: 21 additions & 24 deletions src/css/rules/style.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,37 +201,34 @@ impl<R> StyleRule<R> {
// The CSS modules `composes` property is handled specially, and omitted during printing.
// We need to add the classes it references to the list for the selectors in this rule.
if let Property::Composes(composes) = decl {
if dest.is_nested() && dest.css_module.is_some() {
// `Property::parse` makes this property only in a CSS module.
debug_assert!(dest.css_module.is_some());
if dest.is_nested() {
return dest.new_error(
PrinterErrorKind::invalid_composes_nesting,
Some(composes.cssparser_loc),
);
}

if dest.css_module.is_some() {
// `handle_composes` needs `&mut dest` while the
// module also lives in `dest.css_module`. Move the
// module out for the duration of the call, then put
// it back before any `dest.new_error` early return.
let mut cm = dest.css_module.take();
let err = if let Some(css_module) = &mut cm {
css_module
.handle_composes(
dest,
&self.selectors,
composes,
self.loc.source_index,
)
.err()
} else {
None
};
dest.css_module = cm;
if let Some(error_kind) = err {
return dest.new_error(error_kind, Some(composes.cssparser_loc));
}
continue;
// Move the module out of `dest`: `handle_composes` also takes `&mut dest`.
let mut cm = dest.css_module.take();
let err = if let Some(css_module) = &mut cm {
css_module
.handle_composes(
dest,
&self.selectors,
composes,
self.loc.source_index,
)
.err()
} else {
None
};
dest.css_module = cm;
if let Some(error_kind) = err {
return dest.new_error(error_kind, Some(composes.cssparser_loc));
}
continue;
Comment on lines 203 to +231

@claude claude Bot Oct 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟣 pre-existing, not blocking: pre-existing nit: a minified CSS module whose rule has a composes declaration gets a stray ; before the closing } (.a_x{color:red;}). len at src/css/rules/style.rs:168 counts the Composes entries, but the continue at src/css/rules/style.rs:231 skips the i += 1 at style.rs:241, so for the last printed declaration i != len - 1 is always true and the ; at style.rs:238 is written even with dest.minify. Fix: count only printed declarations (e.g. compute len excluding Property::Composes, or decide the trailing ; by whether another printable declaration follows), so every minified module rule with composes prints without the trailing separator.
A small fix can ride a push you are already making; otherwise a short reply is enough.

Why this was flagged

A CSS module such as .a { composes: b; color: red } is bundled with minify (bun build --minify or Bun.build minify.whitespace) so dest.minify is true. In StyleRule::to_css_base, len at src/css/rules/style.rs:168-169 is 2 (declarations plus important declarations, composes included). The Property::Composes entry hits the continue at src/css/rules/style.rs:231 after handle_composes, so i stays 0. For color: red, the check at src/css/rules/style.rs:236 evaluates i != len - 1 as 0 != 1, true, and ; is written at style.rs:238, giving .a_x{color:red;} instead of .a_x{color:red}. The base branch behaves the same for CSS modules (its continue ran whenever dest.css_module.is_some()), so this is pre-existing; the diff touches this exact continue and reorganizes the arm, which is why it is reported here. No test covers minified output of a module rule with composes.

Verification: pre-existing (nit-sized): triggered whenever a CSS module rule contains a composes declaration and the printer runs with dest.minify. In /home/claude/bun/src/css/rules/style.rs, len at lines 168-169 counts Property::Composes entries; the Composes arm ends with continue; (line 231), which jumps past i += 1 at line 241, so ; is written at line 238. The base branch had the identical continue;, so the behavior is unchanged by this PR.

@robobun robobun Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Confirmed, and it is the same without this change. bun build --minify of a CSS module with .a { composes: b; color: red } prints .a_5WBUVQ{color:red;} on 1.4.3-canary (367d939d9). len counts the Composes entry, and the continue skips i += 1, as you describe.

This PR does not change it. It leaves the output of a CSS module byte-identical, and this fix changes minified CSS module output. It needs its own change, with a test for a minified module rule that has composes.

}

dest.newline()?;
Expand Down
Loading
Loading