Skip to content

Strip a leading UTF-8 BOM when parsing a style sheet - #1301

Open
X-Coder264 wants to merge 1 commit into
parcel-bundler:masterfrom
X-Coder264:strip-leading-bom
Open

Strip a leading UTF-8 BOM when parsing a style sheet#1301
X-Coder264 wants to merge 1 commit into
parcel-bundler:masterfrom
X-Coder264:strip-leading-bom

Conversation

@X-Coder264

Copy link
Copy Markdown

Fixes #338.

A leading UTF-8 BOM was tokenised as part of the CSS instead of being removed while decoding the input. U+FEFF is a valid ident code point, so it gets glued onto whatever follows, which produces two different failures:

input 1.33.0 this PR
 + html{color:red} html{color:red} — parses, but the selector is now html and matches nothing html{color:red}
 + /*! license */html{color:red} Unexpected token Ident("html") at 1:2 /*! license */\nhtml{color:red}
 + @charset "UTF-8";html{color:red} Unexpected token AtKeyword("charset") at 1:2 html{color:red}

The first row is the one I found most alarming: no error, no warning, the rule is just silently dropped from the page. When a comment or at-rule follows the BOM, the same lexing yields two idents separated by non-whitespace, which is the parse error people actually report — and it is hard to diagnose, because the location is 1:2 of a generated bundle and the identifier in the message is the first selector of the first stylesheet, often dozens of KB away from the cause.

CSS Syntax §3.2 removes the BOM when decoding the input byte stream, before tokenisation, so this strips it in StyleSheet::parse_with — the single choke point shared by the Rust API, the Node/WASM/C bindings and the bundler. Because the bundler parses every file through it, imported files are covered too, not just the entry.

Why this is worth fixing now

The BOM is no longer only an artefact of hand-authored files saved in an editor: it is showing up in generated CSS. Dart Sass prepends a BOM (instead of @charset "UTF-8") when compressed output contains non-ASCII characters, e.g. any project importing bootstrap-sass glyphicons. Until postcss 8.5.23 a postcss pass silently dropped it; postcss 8.5.24 changed to "Preserve the BOM after the processing", so it now reaches the minifier and breaks webpack/Parcel production builds that use lightningcss. That is how I ran into it.

Decisions I would like your call on

  • The BOM is not re-emitted. That matches how lightningcss already discards @charset rules; a consumer that needs one can prepend it.
  • Source maps. The bundler also strips before set_source_content, so sourcesContent lines up with the positions the parser reports. The same one-line change would be needed in napi/src/lib.rs, src/main.rs and c/src/lib.rs for their source maps to be exact for BOM'd input (as it stands, line 1 columns would be one character to the left). I left those out to keep the diff focused, since it would mean either duplicating the helper or making strip_bom public — happy to do whichever you prefer.
  • StyleAttribute::parse is unchanged. An inline style attribute is not a decoded byte stream, so a U+FEFF there is an ordinary character rather than an encoding marker.
  • Cost is one strip_prefix per style sheet at parse entry, nothing per token, so the benchmark concern raised in UTF-8 BOM Handling #338 shouldn't apply.

Tests

  • tests::test_bom in src/lib.rs — leading BOM followed by a rule, a /*! license comment, @charset and @import, plus a non-leading BOM which must survive as an ordinary character.
  • bundler::tests::test_bom — BOM on both the entry and an imported file.

cargo test --all-features passes with RUSTFLAGS=-D warnings. My added code is rustfmt-clean; note that running cargo fmt on current master also reformats some pre-existing code in src/selector.rs, src/bundler.rs and src/lib.rs, which I reverted so this diff stays focused (it is purely additive).

I could not run yarn test — no npm registry access in my environment — so there is no JS-level test here. Happy to add one to node/test/transform.test.mjs if you'd like it covered there too.

U+FEFF is a valid ident code point, so a leading BOM was tokenised as part
of the CSS: `BOM html {}` silently parsed as a `\u{feff}html` type selector
that matches nothing, and `BOM /*! license */ html {}` failed to parse with
`Unexpected token Ident("html")`. CSS Syntax removes the BOM while decoding
the input byte stream, before tokenisation, so strip it in `parse_with`,
which every consumer and every bundled file goes through.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UTF-8 BOM Handling

1 participant