Skip to content

fix(compiler): preserve TSRX expression and comment spans - #3787

Merged
ryansolid merged 3 commits into
solidjs:nextfrom
everton-dgn:fix/tsrx-parenthesized-key-spans
Oct 5, 2026
Merged

ryansolid merged 3 commits into
solidjs:nextfrom
everton-dgn:fix/tsrx-parenthesized-key-spans

Conversation

@everton-dgn

@everton-dgn everton-dgn commented Oct 4, 2026 •

Copy link
Copy Markdown

Native TSRX compilation rejected keys such as key(item.id) and annotated @for loops used directly as a component body. Deeply parenthesized iterables and conditions could also leave parser scaffolds behind. Keep exact aliases for authored parentheses and use the authored iterable to find annotated loop anchors.

This also fixes projected comments reaching code generation with offsets into the wrong source. With source maps enabled, that could produce invalid JavaScript or panic when an offset split a Unicode character. Discard scaffold comments and map authored comments back to the original source before code generation.

The parser revision, supported grammar, and Rust 1.95 minimum stay unchanged.

Validation:

  • 6,028 compiler JS tests passed; 2 skipped. The 85 focused tests also passed after formatting, covering DOM, SSR, the universal renderer, key evaluation, list updates, and comment offset sweeps.
  • Rust tests passed with default features (61), no default features (12), and TSRX without Node bindings (81).
  • Integration with fix(compiler): preserve TSRX setup spans at synthetic semicolons #3779 and the compatible parser backport passed 226 cases: 213 valid programs and 13 expected rejections. Every generated program parsed successfully, and each source map retained the authored source. The combined compiler also passed 57 Rust unit tests.
  • Clippy and rustfmt passed when combined with chore(compiler): restore Clippy and rustfmt checks #3780, which fixes the existing lint and formatting failures on next.

CI: Solid CI, both size checks, and Socket Security passed. CodSpeed flagged one Signals benchmark, mount 4000 rows / memo + sync render effect only (reference), at 26.9 ms versus 29.7 ms (-9.42%); the other 187 were unchanged. The report warns that the runtime environments differ. The Signals source tree, benchmark, build configuration, and lockfile are identical between the base and this commit, and the measured code does not call the TSRX compiler. That does not establish identical remote artifacts or environments. I could not rerun the workflow because GitHub requires repository admin access, so the performance result remains unresolved.

@changeset-bot

changeset-bot Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 4e3b938

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 12 packages
Name Type
@solidjs/compiler Patch
@solidjs/babel-plugin Patch
@solidjs/diagnostics Patch
@solidjs/element Patch
@solidjs/h Patch
@solidjs/html Patch
@solidjs/signals Patch
solid-js Patch
@solidjs/universal Patch
@solidjs/web Patch
test-integration Patch
todos-server-example Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@codspeed

codspeed Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 188 untouched benchmarks


Comparing everton-dgn:fix/tsrx-parenthesized-key-spans (4e3b938) with next (ea5f1da)

Open in CodSpeed

@everton-dgn

everton-dgn commented Oct 5, 2026 •

Copy link
Copy Markdown
Author

I checked the 4,000-row mount regression between 1a3f87f and 650dfb1. The benchmark loads @solidjs/signals from dist/prod. The Signals sources, build configuration, and lockfile are identical in those commits; the compiler files changed by this PR are outside that build path.

Independent Linux builds produced identical output for all 79 files in dist, including all 36 in dist/prod. I also ran two CodSpeed 4.19.1 simulation comparisons on Linux ARM64, with Node 24.21.0 and the unmodified 5.4.0 Vitest plugin. For the flagged reference case, the instruction counts were:

Comparison Base Head Change
First pair 59,323,707 56,065,791 -5.49%
Second pair, reversed order 59,381,949 59,086,707 -0.50%

Neither local pair reproduced the reported slowdown. This does not clear the remote alert: CI uses a different environment, and its runs have no downloadable build artifacts to compare.

The fresh CI run passed. CodSpeed now reports 188 unchanged benchmarks, and all seven PR checks are green. Commit c64668b is an empty CI revalidation commit with the same tree as 650dfb1.

@ryansolid
ryansolid merged commit c8aac88 into solidjs:next Oct 5, 2026
7 checks passed
@ryansolid

Copy link
Copy Markdown
Member

Merged — thanks for the TSRX expression and comment span fixes! I merged next into the branch first to pick up #3779; the only conflict was in tsrx/leaf.rs, where LeafMap::new now also takes the projected source, and the tests from both PRs are kept.

— Claude via Cursor

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.

2 participants