Repository navigation
Conversation
|
Updated 7:36 AM PT - Oct 16th, 2026
⏳ @robobun, your commit 1b1fff6 is still building in |
|
Status Reproduced on bun 1.4.2 and on main:
The fix and the tests are in this PR (#45140). 22 of the 27 new test cases fail on main and all 27 pass with the fix. The branch is rebased onto main 5866b5f. The transpiler cache version is now 36, because main took 35 in #45290. CI. The diff is green. Build 126704 finished with one failing test: |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 6 seconds. View limit details
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe lexer now stops JSX pragma values at JavaScript whitespace, including line breaks, and returns no span for empty values. The runtime transpiler cache format version changes from 35 to 36. New transpiler and bundler tests cover pragma parsing and JSX output. ChangesJSX pragma parsing
Suggested reviewers: Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to
|
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline finding, I also checked that strings::is_js_whitespace is a strict superset of the old is_whitespace stop set (0xFEFF and Unicode Zs route through lexer::is_whitespace), so the // path only changes in the value-less case; and that the new None return (offset 0) cannot loop in either scanner, since both the block-comment index_of_any loop and scan_single_line_comment advance past the @/# before calling scan_pragma. The empty-text path (/* @ jsx */) stops at i == 0 without reading past the slice.
Extended reasoning...
The PR changes PragmaArg::scan in src/js_parser/lexer.rs so a JSX pragma value ends at any JS whitespace including line terminators in both comment kinds, deletes CommentKind/is_newline/allow_newline, returns None for an empty value, and bumps the transpiler cache EXPECTED_VERSION to 35; tests cover LF/CRLF/CR/U+2028/U+2029 through Bun.Transpiler, itBundled and spawned bun run. No security-sensitive surface. Not approved because it is a deliberate semantic deviation from esbuild in core lexer code with a behavior change for bare-name pragmas, and one confirmed finding about the pre-existing # marker handling is posted inline.
A JSX pragma (`@jsx`, `@jsxFrag`, `@jsxRuntime`, `@jsxImportSource`) on a line of its own in a `/* */` comment kept the line break in its value. `PragmaArg::scan` ended the value at a line break only for `//` comments. The value now ends at white space or at a line break in both comment kinds. A pragma name with no value is not a pragma, so it does not reset the value of an earlier pragma. The runtime transpiler cache version goes from 35 to 36, so that output cached by an older build is not used again.
a84567a to
d76fd2e
Compare
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
LGTM — a narrow lexer change: the pragma value loop now stops at strings::is_js_whitespace, which for // comments is exactly the old is_whitespace || is_newline stop set and for block comments adds the four line terminators; the new None on an empty value keeps an earlier pragma instead of blanking it, and both callers tolerate a 0 return without re-scanning the same marker. Cache version bump is in place (the description's "34 to 35" is stale; the code goes 35 to 36).
Extended reasoning...
The change touches src/js_parser/lexer.rs (PragmaArg::scan, scan_pragma and its two call sites, deletion of CommentKind/is_newline/allow_newline) and bumps EXPECTED_VERSION in src/jsc/RuntimeTranspilerCache.rs, with new exact-value tests in test/bundler/transpiler/transpiler.test.js and test/bundler/bundler_jsx.test.ts covering LF/CRLF/CR/U+2028/U+2029, name-only pragmas, and end-to-end bun run/bun build. It touches no security-sensitive surface. Approve was decided by the small, self-contained diff whose stop-set change I traced against is_whitespace/is_js_whitespace, by JSXPragma accessors already gating on text.len() > 0 so the zero-length-span removal is behavior-neutral for consumers, and by no CODEOWNERS entry covering the changed paths; the only inline finding is a test-assertion-ordering nit. The earlier inline thread about the # marker was a pre-existing, non-blocking note the author replied to.
Problem
/* */comment keeps the line break in its value.bun format --insert-pragmawrites that form. Then@jsxImportSource ./myfails withCannot find module './my\n/jsx-dev-runtime'.PragmaArg::scan(src/js_parser/lexer.rs:3552) stops at a line break only for//comments. Fuzzing found this. No issue is filed.Fix
allow_newline,CommentKind,PragmaArg::is_newline.test/bundler/bundler_jsx.test.tsandtest/bundler/transpiler/transpiler.test.js(22 of 27 new cases fail on main). Also ranreact-compiler.test.ts,module-graph.test.ts,esbuild/default.test.ts.Background
/** @jsx h */.@or#marker.Downsides
// @jsxwith no value no longer resets an earlier pragma. A value on the line after@jsxis still not read, unlike TypeScript.Notes
Timing.
bun formatis not released yet (#44805, LATEST is 1.4.3). On main it turns a file that runs into a file that does not run. So this is worth landing before the release that shipsbun format.Prettier 3.6.2
--insert-pragmawrites the same four lines.Symptoms on main, per pragma and entry point (
\nis a real line break in the value):bun runbun buildBun.Transpiler@jsxImportSource ./myCannot find module './my\n/jsx-dev-runtime'Could not resolve"./my\n/jsx-dev-runtime"@jsxRuntime classicUnsupported JSX runtime: "classic\n", the automatic runtime is used@jsx h(classic runtime)ReferenceError: h is not definedwhenhis an import in a.tsxfileReferenceError: h is not definedh\n("a", null)@jsxFrag F(classic runtime)FFF\nA pragma at the start of the next line was also swallowed:
/*\n@jsxRuntime classic\n@jsx h\n*/gaveUnsupported JSX runtime: "classic\n@jsx". The same happened to a@__PURE__at the start of the next line. LF, CRLF, a lone CR, U+2028 and U+2029 all behave the same, with or without a leading*.Published code. 11 files in
dist/esmof@atlaskit/primitives22.5.4 have a docblock with@jsxRuntime classicand@jsx jsxon their own lines.bun build node_modules/@atlaskit/primitives/dist/esm/index.js --packages externalprints 11Unsupported JSX runtime: "classic\n"warnings on main and 0 with this PR. The two bundles are byte-identical, because those files contain no JSX.One earlier sighting. The output pasted in #10026 (closed, about another crash) shows
Unsupported JSX runtime: "theRuntime.` followed by a line break. That text is prose in a doc comment of estree-util-build-jsx. With this PR the warning still prints, without the line break.What changes for
//comments. Only the value-less case. The stop set of the loop is the same as before:strings::is_js_whitespaceisis_whitespaceplus LF, CR, U+2028 and U+2029. A name with no value (// @jsxat a line end,/* @jsx */) returned an empty value before, and that reset the value of an earlier pragma of the same name. It now returns nothing, asscanForPragmaArgin esbuild does. The tests cover this for all four names.Other tools, run on this machine. esbuild 0.21.5 keeps the line break (
Invalid JSX factory: h\n,Invalid JSX runtime: "classic\n"). TypeScript 6.0.2 and swc 1.3.38 follow the pragma on its own line. For/* @jsx<LF> h */, TypeScript usesh, esbuild and swc do not.Measurements. Release builds of main d1cb7db and of this PR on that base, linux x64. The branch was rebased after these runs and the lexer change is the same. valgrind, perf, strace and bloaty are not installed on this machine.
size, main d1cb7db vs PR);PragmaArg::scan3,025 -> 2,305 B,scan_pragma662 -> 660 B,scan_comment_text1,088 -> 1,069 B,scan_single_line_comment508 -> 508 B,Lexer::next15,420 -> 15,420 B (nm -S)PragmaArg::scancalls while transpiling typescript.js / lodash.js / bun.d.ts: 0 / 0 / 0 (base) and 0 / 0 / 0 (PR), against 1,022 / 3,695 / 1,080scan_pragmacalls (gdb breakpoint hit counts). The output ofbun build --no-bundleis byte-identical for the three files.scan_pragmaon a non-JSX marker: 264 -> 262 instructions per call for/** @param ... */, 264 -> 262 for// @param ...(gdb single-step count, base vs PR)scan_pragmaon a matched pragma:/** @jsx h */285 -> 277,// @jsx h329 -> 302,/** @jsxImportSource @emotion/react */776 -> 742, multi-line@jsx h316 -> 279 instructions (gdb single-step count, base vs PR)PragmaArg::scanandscan_pragma(base 0, PR 0; objdump call targets)//forms and two pragmas on one line); 0 expectations edited in existing testsLeft alone.
/* @jsx<LF> h */: a value on the line after the name is not read. A test holds this.#marker.#jsx his read like@jsx h, which esbuild and TypeScript do not do. A comment such as// see https://www.typescriptlang.org/tsconfig#jsxImportSource for detailssets the import source tofor, before and after this PR. That is a separate change. It is reported separately and is not part of this PR.bun formatwithprettier-plugin-organize-importsremoves the import of the factory when@jsxRuntimecomes before@jsxin one comment (src/format/js/sort_imports/organize.rs:1514). The one-line form fails too, so it is not a line-break fault. It is reported separately and is not part of this PR.bun check(src/sema/parser/pragmas.rs:138) and the React rules ofbun lint(src/lint/react/util_pragma.rs:27).Bun.Transpilerthrows on a warning (Bun.Transpiler: do not throw on a parser warning #42433). Not changed here.Bun.Transpiler#scanImportsapplies no JSX pragma at all, the one-line forms included.has_bun_pragma(src/js_parser/parse/parse_entry.rs:2542) is a separate reader for the first-line// @buncomment, which Bun writes itself.Self-review. It asked for: the symptoms per pragma and entry point, the formatter route, fixtures where the factory is an import and one process per pragma, the value-less rule tested for all four names, a no-JSX case that does not depend on
Bun.Transpilerthrowing on a warning, and the full list above. All are in this PR. It also offered to fold the#marker fix into this PR. That stays separate, because it is a different behaviour change with its own repro.Related PRs. #39636 changes the signatures of
scan_pragmaandPragmaArg::scan, so the second of the two to land needs a small rebase. #42676 says its member-expression rule cannot cover the@jsxand@jsxFragpragmas until the lexer ends the value at the line end. The//stop that this PR makes general came with #19134.Suites run. Debug build:
bundler_jsx.test.ts(77 pass),transpiler.test.js(252 pass),react-compiler.test.ts(116 pass). Release build of this PR:module-graph.test.ts(292 pass),transpiler-cache.test.ts(20),esbuild/default.test.ts(155),esbuild/dce.test.ts(87),esbuild/tsconfig.test.ts(21),bundler_comments.test.ts(50), andbun-build-api.test.ts(71 pass, 1 fail:bytecode: repeated builds don't retain the generated code, which fails the same way on the release build of main). On the debug build,module-graph.test.tsandbun-build-api.test.tshave tests that time out at 5 s. They pass on the release builds.