Skip to content

css: require the import record list in StyleSheet::parse and an ImportInfo in StyleSheet::to_css - #47990

Open
robobun wants to merge 2 commits into
farm/ca5ddaa0/no-bundle-css-urlfrom
robobun/9a18a978/css-require-import-records
Open

robobun wants to merge 2 commits into
farm/ca5ddaa0/no-bundle-css-urlfrom
robobun/9a18a978/css-require-import-records

Conversation

@robobun

@robobun robobun commented Nov 6, 2026 •

Copy link
Copy Markdown
Collaborator

Stacked on #38528.

Behaviour change: none

Problem

Fix

  • The three functions take &mut Vec<ImportRecord> and ImportInfo, as parse_bundler and StyleAttribute::parse already do. The 8 callers drop their Some(..).
  • Correct because no caller passes None after bun build --no-bundle: keep css url() references instead of failing to parse #38528. Parser::new and to_css_with_writer keep the Option: An+B, Bun.color and the linker's url-free blocks have no list.
  • Verified: with None put back in build_css_output, cargo check -p bun_bundler gives two E0308 errors. test/bundler/css/, test/js/bun/css/, test/bundler/cli.test.ts and bun-pm-diff.test.ts pass.
  • Self-reviewed: 5 concerns raised, 5 addressed (list in Notes).

Background

  • Import record: the CSS parser adds one bun_ast::ImportRecord per url(). The printer reads the url() back from it through ImportInfo (src/css/printer.rs).
  • Considered a parser that keeps a url() as text with no list: the tests of bun build --no-bundle: keep css url() references instead of failing to parse #38528 pass, but a bundler caller that forgets its table then prints unresolved urls. Considered one parse-and-print entry point in bun_css: a new API, which needs a maintainer's decision.

Downsides

  • None found. The release build differs from its base in one function of 163,450: StyleSheet::parse_with, which parse and parse_bundler call, is 16 bytes smaller. The binary keeps its size.
Notes

Why a separate PR

The base of this PR is the branch of #38528. The repair of bun build --no-bundle is two arguments in one function and is in that PR. This change reaches no user, so it can wait or be dropped without an effect on that repair.

parse_with is the shared body of parse and parse_bundler. It now takes the NonNull pointer to the list that both of them already have, and no longer an Option of it.

Callers

  • StyleSheet::parse: src/bundler/transpiler.rs (build_css_output), src/runtime/cli/pm_diff_normalize.rs (normalize_css), src/css_jsc/css_internals.rs (testing_impl).
  • StyleSheet::to_css: the same three, and src/bundler/linker_context/prepareCssAstsForChunk.rs.
  • StyleAttribute::to_css: src/css_jsc/css_internals.rs (attr_test).
  • None is in cfg-gated code, so the host build compiles each of them.

What keeps the Option

  • Parser::new(.., None, ..): the An+B parser in src/css/css_parser.rs and Bun.color in src/css_jsc/color_js.rs. These grammars have no url().
  • to_css_with_writer(.., None, ..): the linker prints url-free blocks with it in src/bundler/linker_context/generateCompileResultForCssChunk.rs.
  • Printer::new(.., None, ..): Bun.color.

What this does not guard

The text fallback, built and run

Open PRs that edit the same lines

Self-review

  1. Keep this change out of bun build --no-bundle: keep css url() references instead of failing to parse #38528, which repairs the bug. Done: this PR is stacked on it.
  2. Say what the types guard and what they do not. See the two lists above.
  3. Name the open PRs that edit the same lines. See above.
  4. The reason against the text fallback was its cost in a bundled build, and that cost is one compare. I built the fallback and ran it. The reasons are now the two results above.
  5. The doc comment of Parser::new changes by one line.

Measurements (release builds of #38528 and of this branch, linux x64)

  • Size: stripped bun is 140,478,752 bytes on both. .text is 110,627,497 bytes on both.
  • Of 163,450 sized text symbols (llvm-nm -S), 1 differs: StyleSheet::parse_with::<BundlerAtRuleParser>, 9,143 to 9,127 bytes and 1,719 to 1,715 instructions. It no longer tests the Option.
  • StyleSheet::parse (282 bytes), parse_bundler (282), to_css_with_writer (1,535), StyleAttribute::to_css (1,231) and Transpiler::build_css_output (2,566) are equal on both.
  • Compile check: with None put back at the two arguments in build_css_output, cargo check -p bun_bundler prints error[E0308]: mismatched types: expected &mut Vec<ImportRecord<'_>>, found Option<_> and error[E0308]: mismatched types: expected ImportInfo<'_>, found Option<_>.
  • The 88 probe stylesheets that I ran under --no-bundle print the same bytes on both debug builds.

Suites run on the debug build of this branch

test/bundler/cli.test.ts, test/bundler/css/, test/js/bun/css/, test/bundler/esbuild/css.test.ts, test/cli/install/bun-pm-diff.test.ts, test/bundler/bundler_loader.test.ts, test/bundler/bundler_html.test.ts: 3,956 pass, 48 skip, 4 todo, 0 fail.


no test proof · iteration 2 · the description declares no behaviour change, so there is no failing test to prove; the existing suite in CI is the check

…tInfo in to_css

StyleSheet::parse took Option<&mut Vec<ImportRecord>>. StyleSheet::to_css and
StyleAttribute::to_css took Option<ImportInfo>. No caller has a reason to pass
None: a style sheet can hold a url(), the parser rejects a url() when there is
no list, and the printer cannot print one with no table. Two callers passed
None by mistake: build_css_output (both arguments, #38528) and bun pm diff
(the print side, #45307).

The three signatures now take the value itself, as parse_bundler and
StyleAttribute::parse already do. A caller that leaves the list out does not
compile. parse_with takes the NonNull that both of its callers already have.
The callers drop their Some(..). No behaviour change.

Parser::new and to_css_with_writer keep the Option: a parser of one value
(An+B, a color) and the linker's url-free blocks have no list.
@robobun

robobun commented Nov 6, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 8:06 PM PT - Nov 8th, 2026

❌ @robobun, your commit 2bd274a has some failures in Build #138973 (All Failures)


🧪   To try this PR locally:

bunx bun-pr 47990

That installs a local version of the PR into your bun-47990 executable, so you can run:

bun-47990 --bun

@robobun

robobun commented Nov 6, 2026 •

Copy link
Copy Markdown
Collaborator Author

This PR changes three signatures and no behaviour, so there is no failing input to reproduce.

What it prevents: with None put back at the two arguments in Transpiler::build_css_output, cargo check -p bun_bundler fails:

src/bundler/transpiler.rs:3063:13: error[E0308]: mismatched types: expected `&mut Vec<ImportRecord<'_>>`, found `Option<_>`
src/bundler/transpiler.rs:3092:13: error[E0308]: mismatched types: expected `ImportInfo<'_>`, found `Option<_>`

Before this PR that code compiles, and bun build --no-bundle x.css then fails on each url() (#38528).

This PR (#47990) is stacked on #38528.

CI on 2bd274a (build 138973): no test failed. Each test lane that ran passed all of its shards: Linux (alpine, debian, debian with ASAN, ubuntu, on x64 and aarch64), Windows (x64 and aarch64) and macOS aarch64. The four macOS x64 test shards did not run. Each one waited in the queue without an agent until it expired, four times in a row.

@claude claude Bot left a comment

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.

Code review found no issues

No high-confidence issues detected in this change.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants