Skip to content

css: keep the namespace prefix on attribute selectors that have an operator #45159

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: keep the namespace prefix on attribute selectors that have an operator #45159

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
dfdbcbc
css: keep the namespace prefix on attribute selectors that have an op…
robobun Oct 13, 2026
1e3da36
css: make the namespace of the boxed attribute selector node required
robobun Oct 13, 2026
790e5aa
test(css): assert the error for a late @namespace, run the pm diff ca…
robobun Oct 16, 2026
e411703
ci: retrigger
robobun Oct 17, 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
64 changes: 26 additions & 38 deletions src/css/selectors/parser.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,26 +182,24 @@ pub mod attrs {
}

#[derive(Clone)]
pub struct AttrSelectorWithOptionalNamespace<Impl: SelectorImpl> {
pub(crate) namespace: Option<NamespaceConstraint<NamespaceUrl<Impl>>>,
pub struct AttrSelectorWithNamespace<Impl: SelectorImpl> {
pub(crate) namespace: NamespaceConstraint<NamespaceUrl<Impl>>,
pub(crate) local_name: Impl::LocalName,
pub(crate) local_name_lower: Impl::LocalName,
pub(crate) operation: ParsedAttrSelectorOperation<Impl::AttrValue>,
pub(crate) never_matches: bool,
}

impl<Impl: BunSelectorImpl> AttrSelectorWithOptionalNamespace<Impl> {
impl<Impl: BunSelectorImpl> AttrSelectorWithNamespace<Impl> {
pub fn to_css(&self, dest: &mut Printer) -> Result<(), PrintErr> {
dest.write_char(b'[')?;
if let Some(nsp) = &self.namespace {
match nsp {
NamespaceConstraint::Specific(v) => {
IdentFns::to_css(&v.prefix, dest)?;
dest.write_char(b'|')?;
}
NamespaceConstraint::Any => {
dest.write_str("*|")?;
}
match &self.namespace {
NamespaceConstraint::Specific(v) => {
IdentFns::to_css(&v.prefix, dest)?;
dest.write_char(b'|')?;
}
NamespaceConstraint::Any => {
dest.write_str("*|")?;
}
}
IdentFns::to_css(&self.local_name, dest)?;
Expand Down Expand Up @@ -231,31 +229,23 @@ pub mod attrs {
}

pub fn eql(&self, rhs: &Self) -> bool {
match (&self.namespace, &rhs.namespace) {
(None, None) => true,
(Some(a), Some(b)) => a.eql(b),
_ => return false,
}
.then_some(())
.is_some()
self.namespace.eql(&rhs.namespace)
&& self.local_name.eql(&rhs.local_name)
&& self.local_name_lower.eql(&rhs.local_name_lower)
&& self.operation.eql(&rhs.operation)
&& self.never_matches == rhs.never_matches
}
pub(crate) fn deep_clone(&self) -> Self {
Self {
namespace: self.namespace.as_ref().map(|n| n.deep_clone()),
namespace: self.namespace.deep_clone(),
local_name: self.local_name,
local_name_lower: self.local_name_lower,
operation: self.operation.deep_clone(),
never_matches: self.never_matches,
}
}
pub(crate) fn hash(&self, hasher: &mut Wyhash) {
if let Some(ns) = &self.namespace {
ns.hash(hasher);
}
self.namespace.hash(hasher);
self.local_name.hash(hasher);
self.local_name_lower.hash(hasher);
self.operation.hash(hasher);
Expand Down Expand Up @@ -1981,16 +1971,16 @@ pub enum GenericComponent<Impl: SelectorImpl> {
local_name: Impl::LocalName,
local_name_lower: Impl::LocalName,
},
/// Used only when local_name is already lowercase.
/// `[name op value]` with no namespace prefix. The name can have any letter case.
AttributeInNoNamespace {
local_name: Impl::LocalName,
operator: attrs::AttrSelectorOperator,
value: Impl::AttrValue,
case_sensitivity: attrs::ParsedCaseSensitivity,
never_matches: bool,
},
/// Use a Box in the less common cases with more data to keep size_of::<Component>() small.
AttributeOther(Box<attrs::AttrSelectorWithOptionalNamespace<Impl>>),
/// An `ns|` or `*|` attribute selector, in a Box to keep size_of::<Component>() small.
AttributeOther(Box<attrs::AttrSelectorWithNamespace<Impl>>),

/// Pseudo-classes
Negation(Box<[GenericSelector<Impl>]>),
Expand Down Expand Up @@ -3433,8 +3423,8 @@ pub(crate) fn parse_attribute_selector<Impl: BunSelectorImpl>(
// [foo]
let local_name_lower: *const [u8] = arena_lowercase(input.arena(), local_name);
if let Some(ns) = namespace {
let x = attrs::AttrSelectorWithOptionalNamespace::<Impl> {
namespace: Some(ns),
let x = attrs::AttrSelectorWithNamespace::<Impl> {
namespace: ns,
local_name: Ident { v: local_name },
local_name_lower: Ident {
v: local_name_lower,
Expand Down Expand Up @@ -3505,7 +3495,7 @@ pub(crate) fn parse_attribute_selector<Impl: BunSelectorImpl>(
let attribute_flags = parse_attribute_flags(input)?;

let value: Impl::AttrValue = std::ptr::from_ref::<[u8]>(value_str);
let (local_name_lower, local_name_is_ascii_lowercase): (Impl::LocalName, bool) = 'brk: {
let local_name_lower: Impl::LocalName = 'brk: {
let first_uppercase = 'a: {
for (i, &b) in local_name.iter().enumerate() {
if b >= b'A' && b <= b'Z' {
Expand All @@ -3517,21 +3507,19 @@ pub(crate) fn parse_attribute_selector<Impl: BunSelectorImpl>(
if let Some(first_uppercase) = first_uppercase {
let str_ = &local_name[first_uppercase..];
let lowered: *const [u8] = arena_lowercase(input.arena(), str_);
break 'brk (Ident { v: lowered }, false);
break 'brk Ident { v: lowered };
Comment thread
robobun marked this conversation as resolved.
Resolved
} else {
break 'brk (
Ident {
v: std::ptr::from_ref::<[u8]>(local_name),
},
true,
);
break 'brk Ident {
v: std::ptr::from_ref::<[u8]>(local_name),
};
}
};
let case_sensitivity: attrs::ParsedCaseSensitivity =
attribute_flags.to_case_sensitivity(local_name_lower.v(), namespace.is_some());
if namespace.is_some() && !local_name_is_ascii_lowercase {
// lightningcss boxes uppercase names (`|| !local_name_is_ascii_lowercase`) for its matcher.
if let Some(namespace) = namespace {

@claude claude Bot Oct 13, 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.

🔴 Users who bundle several CSS files into one chunk can lose whole rules after merging that still applied on the base branch. Line 3520 now always keeps the xl|/*| prefix, but the linker neither hoists @ namespace ahead of earlier files' rules nor keeps it out of @ media/@ supports/@ layer wrappers (prepareCssAstsForChunk.rs:371, :424), so browsers ignore the declaration and drop every rule using the prefix, merged selector lists included. Fix: in the CSS linker, hoist each file's @ namespace rules to the chunk head and never wrap them in condition blocks, or at least emit a warning like esbuild's "@ namespace" rules are not supported so the drop is not silent.

Why this was flagged

A JS entry imports base.css (.icon{fill:red}) and then icons.css (@ namespace xl "urn:example:xlink"; a[href^="#"]{color:green} a[xl|href^="#"]{color:green}), or icons.css is reached through @ import url(icons.css) screen. prepareCssAstsForChunk.rs:369-378 strips only leading @ import/ignored rules and keeps @ layer statements; no code moves @ namespace, and wrap_rules_with_conditions (prepareCssAstsForChunk.rs:424, :497-547) nests the whole file, @ namespace included, inside @ layer/@ supports/@ media blocks. Browsers honour @ namespace only before every style rule and never inside a block, so the prefix xl is undeclared where the rules print. On the base branch both rules printed as a[href^="#"] and merged into one valid rule that applied. After src/css/selectors/parser.rs:3520 both print with the prefix; the minifier merges them into a[href^="#"],a[xl|href^="#"]{color:green} and the browser discards the entire rule as an invalid selector list, so the unprefixed a[href^="#"] styling that worked before is lost too. Nothing warns: no @ namespace handling exists under src/bundler/linker_context.

Verification: src/css/selectors/parser.rs:3518-3520 now does if let Some(namespace) = namespace { Ok(GenericComponent::AttributeOther(... so every operator attribute selector with a prefix keeps the prefix. src/bundler/linker_context/prepareCssAstsForChunk.rs:369-378 only strips Import/Ignored and retains LayerStatement; BundlerCssRule::Namespace is never hoisted.

@robobun robobun Oct 16, 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. This is the first Downsides bullet of the PR body. I checked it in Chrome 153 with a JS entry that imports base.css and then icons.css: main keeps 3 rules and this branch keeps 1.

This PR does not change the linker, for two reasons:

  • The defect is the position of @namespace, and it is on main today. In the same chunk main already drops use[xl|href] and xl|a. The parser of Bun rejects that chunk: @namespace rules must come before any other rules except @charset, @import, and @layer.
  • A move of the rule to the chunk head is not correct for every input. A default @namespace changes what the unprefixed type selectors of the other files match, and one prefix with two URLs in two files needs a rename. The first step is a warning, as esbuild has. That is a new diagnostic with its own tests.

The linker change is separate work in progress. The order of the two changes is a maintainer decision. If the silent drop is not acceptable in a release, the linker change can merge first, or both can ship together. The PR body now says this.

@robobun robobun Oct 20, 2026 •
edited
Loading

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.

The linker change is now #46232 (draft).

  • It prints each misplaced @namespace rule once, after the @import rules of the chunk and outside each condition block.
  • It warns when one chunk cannot hold what the files declare. The cases: one prefix with two URLs, and a default namespace that the files with selectors do not share. It does not rename a prefix.
  • I ran the two cases of this thread on that branch, which does not have the change of this PR. The JS entry prints @namespace xl "urn:example:xlink";.icon{fill:red} and then the rules of icons.css. The screen import prints @namespace xl "urn:example:xlink";.icon{fill:red}@media screen{...}. A second bun build of each output has no error and no warning.

#46232 is stacked on #46040, which is now on main. Proposed merge order: #46040, #46232, then this PR.

Ok(GenericComponent::AttributeOther(Box::new(
attrs::AttrSelectorWithOptionalNamespace::<Impl> {
attrs::AttrSelectorWithNamespace::<Impl> {
namespace,
local_name: Ident {
v: std::ptr::from_ref::<[u8]>(local_name),
Expand Down
29 changes: 29 additions & 0 deletions test/bundler/css/css-modules.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,35 @@ describe("css", () => {
},
});

itBundled("css-module/AttributeSelectorNamespacePrefix", {
files: {
"index.module.css": /* css */ `
@namespace xl "urn:example:xlink";
.a[xl|href="x"] {
color: red;
}
:global([xl|href="y"]) .b {
color: blue;
}
`,
},
outdir: "/out",
entryPoints: ["/index.module.css"],
onAfterBundle(api) {
const css = api.readFile("/out/index.module.css").replace(/\.([ab])_[A-Za-z0-9_-]+/g, ".$1_HASH");
expect(css).toEqualIgnoringWhitespace(`
/* index.module.css */
@namespace xl "urn:example:xlink";
.a_HASH[xl|href="x"] {
color: red;
}
[xl|href="y"] .b_HASH {
color: #00f;
}
`);
},
});

itBundled("css-module/BundleTwoFilesWithoutCodeSplitting", {
files: {
"/foo-entry.js": `
Expand Down
42 changes: 42 additions & 0 deletions test/bundler/esbuild/css.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,48 @@ describe("bundler", () => {
},
});

const namespacedAttributeSelectors = /* css */ `
@namespace xl "urn:example:xlink";
[xl|href$=".pdf"] { color: red }
[href$=".pdf"] { color: gray }
[*|href^="http"] { color: blue }
use[xl|href="#i"] { color: green }
a[xl|href] { color: black }
`;

itBundled("css/AttributeSelectorNamespacePrefix", {
files: {
"/entry.css": namespacedAttributeSelectors,
},
outdir: "/out",
onAfterBundle(api) {
api.expectFile("/out/entry.css").toEqualIgnoringWhitespace(/* css */ `
/* entry.css */
@namespace xl "urn:example:xlink";
[xl|href$=".pdf"] { color: red; }
[href$=".pdf"] { color: gray; }
[*|href^="http"] { color: #00f; }
use[xl|href="#i"] { color: green; }
a[xl|href] { color: #000; }
`);
},
});

itBundled("css/AttributeSelectorNamespacePrefixMinified", {
files: {
"/entry.css": namespacedAttributeSelectors,
},
minifyWhitespace: true,
outdir: "/out",
onAfterBundle(api) {
api
.expectFile("/out/entry.css")
.toEqualIgnoringWhitespace(
`@namespace xl "urn:example:xlink";[xl|href$=".pdf"]{color:red}[href$=\\.pdf]{color:gray}[*|href^="http"]{color:#00f}use[xl|href="#i"]{color:green}a[xl|href]{color:#000}`,
);
},
});

itBundled("css/CSSAtImportMissing", {
files: {
"/entry.css": `@import "./missing.css";`,
Expand Down
Loading
Loading