Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 0 additions & 25 deletions src/css/css_modules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,31 +171,6 @@ impl<'a> CssModule<'a> {
Some(the_hash)
}

pub(crate) fn handle_composes(
&mut self,
_dest: &mut css::Printer,
selectors: &css::selector::parser::SelectorList,
_composes: &css::css_properties::css_modules::Composes,
_source_index: u32,
) -> css::Maybe<(), css::PrinterErrorKind> {
// let bump = dest.arena;
for sel in selectors.v.slice() {
if sel.len() == 1
&& matches!(
sel.components[0],
css::selector::parser::Component::Class(_)
)
{
continue;
}

// The composes property can only be used within a simple class selector.
return Err(css::PrinterErrorKind::invalid_composes_selector);
}

Ok(())
}

pub(crate) fn add_dashed(&mut self, bump: &'a Bump, local: &'a [u8], source_index: u32) {
use bun_collections::array_hash_map::MapEntry;
if let MapEntry::Vacant(v) =
Expand Down
11 changes: 11 additions & 0 deletions src/css/css_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3134,6 +3134,17 @@ impl<'a> Parser<'a> {
}
}

pub(crate) fn mark_import_record_unused(&mut self, import_record_idx: u32) {
let ptr = self
.import_records
.expect("add_import_record only hands out indices when import records are tracked");
// SAFETY: see `Parser.import_records` field doc.
let import_records = unsafe { &mut *ptr.as_ptr() };
import_records[import_record_idx as usize]
.flags
.insert(bun_ast::ImportRecordFlags::IS_UNUSED);
}

#[inline]
pub(crate) fn arena(&self) -> &Bump {
self.input.tokenizer.arena
Expand Down
19 changes: 14 additions & 5 deletions src/css/declaration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -311,10 +311,6 @@ pub(crate) fn parse_declaration<'bump>(
)
}

// Composes handling dispatches through the `ComposesCtx`
// trait (defined in `css_parser.rs`); `NoComposesCtx` returns
// `DisallowEntirely` so the no-tracking fast-path collapses into the match's
// no-op arm.
pub(crate) fn parse_declaration_impl<'bump, C>(
name: &[u8],
input: &mut css::Parser,
Expand Down Expand Up @@ -349,17 +345,28 @@ where

if input.flags.css_modules() {
if let css::Property::Composes(composes) = &mut property {
// `StyleRule::to_css_base` relies on rejected declarations being dropped here.
match composes_ctx.composes_state() {
css::ComposesState::DisallowEntirely => {}
css::ComposesState::Allow(_) => {
composes_ctx.record_composes(composes);
}
css::ComposesState::DisallowEntirely => {
options.warn_fmt(
format_args!("\"composes\" is not valid here"),
source_location.line,
source_location.column,
);
composes.discard(input);
return Ok(());
}
css::ComposesState::DisallowNested(info) => {
options.warn_fmt(
format_args!("\"composes\" is not allowed inside nested selectors"),
info.line,
info.column,
);
composes.discard(input);
return Ok(());
}
css::ComposesState::DisallowNotSingleClass(info) => {
options.warn_fmt_with_notes(
Expand All @@ -373,6 +380,8 @@ where
location: Some(info.to_logger_location(options.filename)),
}]),
);
composes.discard(input);
return Ok(());
}
}
}
Expand Down
10 changes: 0 additions & 10 deletions src/css/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,10 +211,6 @@ pub enum PrinterErrorKind {
},
/// A [std::fmt::Error](std::fmt::Error) was encountered in the underlying destination.
fmt_error,
/// The CSS modules `composes` property cannot be used within nested rules.
invalid_composes_nesting,
/// The CSS modules `composes` property cannot be used with a simple class selector.
invalid_composes_selector,
/// The CSS modules pattern must end with `[local]` for use in CSS grid.
invalid_css_modules_pattern_in_grid,
/// Substituting parent selectors for `&` while compiling CSS nesting for
Expand All @@ -237,12 +233,6 @@ impl fmt::Display for PrinterErrorKind {
bs(*url)
),
Self::fmt_error => f.write_str("Formatting error occurred"),
Self::invalid_composes_nesting => {
f.write_str("The 'composes' property cannot be used within nested rules")
}
Self::invalid_composes_selector => {
f.write_str("The 'composes' property can only be used with a simple class selector")
}
Self::invalid_css_modules_pattern_in_grid => {
f.write_str("CSS modules pattern must end with '[local]' when used in CSS grid")
}
Expand Down
5 changes: 0 additions & 5 deletions src/css/printer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,11 +222,6 @@ impl<'a> Printer<'a> {
b"unknown.css"
}

/// Returns whether the indent level is greater than one.
pub(crate) fn is_nested(&self) -> bool {
self.indent_amt > 2
}

/// Add an error related to std lib fmt errors
pub(crate) fn add_fmt_error(&mut self) -> PrintErr {
self.error_kind = Some(css::PrinterError {
Expand Down
15 changes: 8 additions & 7 deletions src/css/properties/css_modules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ use crate::printer::Printer;

use crate::css_values::ident::{CustomIdent, CustomIdentList};

use crate::dependencies::Location;

use bun_alloc::Arena; // bumpalo::Bump re-export (CSS is an AST/arena crate)
use bun_wyhash::Wyhash;

Expand All @@ -18,13 +16,11 @@ pub struct Composes {
pub from: Option<Specifier>,
/// The source location of the `composes` property.
pub loc: bun_ast::Loc,
pub(crate) cssparser_loc: Location,
}

impl Composes {
pub fn parse(input: &mut Parser) -> css::Result<Composes> {
let loc = input.position();
let loc2 = input.current_source_location();
let mut names = CustomIdentList::default();
while let Ok(name) = input.try_parse(Self::parse_one_ident) {
names.append(name);
Expand All @@ -49,10 +45,16 @@ impl Composes {
loc: bun_ast::Loc {
start: i32::try_from(loc).expect("int cast"),
},
cssparser_loc: Location::from_source_location(loc2),
})
}

/// For a declaration the parser drops: releases the `from "<file>"` import record.
pub(crate) fn discard(&self, input: &mut Parser) {
if let Some(Specifier::ImportRecordIndex(import_record_idx)) = self.from {
input.mark_import_record_unused(import_record_idx);
}
}

pub fn to_css(&self, dest: &mut Printer) -> Result<(), PrintErr> {
use crate::css_values::ident::CustomIdentFns;
dest.write_separated(
Expand Down Expand Up @@ -89,7 +91,6 @@ impl Composes {
names,
from: self.from.as_ref().map(|f| f.deep_clone(bump)),
loc: self.loc,
cssparser_loc: self.cssparser_loc,
}
}

Expand All @@ -107,7 +108,7 @@ impl Composes {
(Some(a), Some(b)) if Specifier::eql(*a, *b) => {}
_ => return false,
}
lhs.loc == rhs.loc && lhs.cssparser_loc == rhs.cssparser_loc
lhs.loc == rhs.loc
}
}

Expand Down
40 changes: 5 additions & 35 deletions src/css/rules/style.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,6 @@ impl<R> StyleRule<R> {
}

fn to_css_base(&self, dest: &mut Printer, is_final_prefix_pass: bool) -> Result<(), PrintErr> {
use css::error::PrinterErrorKind;
use css::properties::Property;

// If supported, or there are no targets, preserve nesting. Otherwise, write nested rules after parent.
Expand Down Expand Up @@ -198,40 +197,11 @@ impl<R> StyleRule<R> {
];
for (decls, important) in decls_groups {
for decl in decls {
// 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() {
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;
}
// `composes` was validated and recorded by the parser (`StyleSheet::composes`).
// Don't re-check nesting here like lightningcss: the bundler wraps imported
// files in their `@import` conditions, so output nesting is meaningless.
Comment thread
robobun marked this conversation as resolved.
if dest.css_module.is_some() && matches!(decl, Property::Composes(_)) {
continue;
}

dest.newline()?;
Expand Down
13 changes: 8 additions & 5 deletions test/bundler/cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -560,21 +560,24 @@ describe.concurrent("modules that fail to print", () => {
});

test("bun build fails instead of emitting a truncated stylesheet when CSS cannot be generated", async () => {
// `composes` on a non-simple selector makes the CSS printer fail; the
// whole stylesheet body used to be dropped while the build exited 0.
// The default browser target compiles nesting away by substituting the
// parent selector for each `&`; with two references per level the printer
// gives up once the expansion limit is hit. The whole stylesheet body used
// to be dropped while the build exited 0.
const depth = 20;
using dir = tempDir("build-css-print-err", {
"styles.module.css": ".b { color: blue }\n.a .c { composes: b; color: red }\n",
"styles.css": "&:is(.a, &.b) {\n".repeat(depth) + "color: red;\n" + "}\n".repeat(depth),
});
await using proc = Bun.spawn({
cmd: [bunExe(), "build", "styles.module.css"],
cmd: [bunExe(), "build", "styles.css"],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).toContain("Failed to generate CSS for this file");
expect(stderr).toContain("styles.module.css");
expect(stderr).toContain("styles.css");
expect(stdout).toBe("");
expect(exitCode).toBe(1);
});
Expand Down
Loading