fix(ast/estree): Fix `JSXOpeningFragment` by therewillbecode · Pull Request #10208 · oxc-project/oxc

therewillbecode · 2025-04-03T19:24:10Z

Fix JSXOpeningFragment node estree TS serialization by skipping selfClosing and attributes fields when AST is TS.

Part of our broader work to align our AST's ESTree output with that of TS-ESLint's. Relates to #9705

graphite-app · 2025-04-03T19:24:17Z

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

0-merge - adds this PR to the back of the merge queue
hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

_{An organization admin has enabled the Graphite Merge Queue in this repository.} _{Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.}

codspeed-hq · 2025-04-03T19:34:45Z

CodSpeed Instrumentation Performance Report

Merging #10208 will not alter performance

_{Comparing therewillbecode:fix/jsx-opening-fragment (4f7958a) with main (48ed6a1)}

Summary

✅ 36 untouched benchmarks

overlookmotel · 2025-04-04T15:14:37Z

Related: #10002

overlookmotel · 2025-04-04T15:35:16Z

We decided not to care about the raw transfer deserializer yet, but on this type we do need to, because it exists in the JS AST, and we don't want to break the existing correct deserialization for JSX (not TSX).

#10238 adds the necessary feature to implement raw_deser correctly for this type. Once it's merged, could you please rebase this PR on top of it? Then I'll add a commit to this PR to fill in raw_deser.

We also have a problem with the TS type defs. This is annoying, but I think we can mis-use the #[ts] attr on meta types to get what we need.

I think you need to:

Add #[estree(add_fields(attributes = JSXOpeningFragmentAttributes, selfClosing = TsFalse))] to the type.
Bring back the JSXOpeningFragmentAttributes meta type. But add a #[ts] attr to it.

That's wrong - these fields aren't TS AST-only, they're JS AST-only. But still I think it'll have the desired effect of adding those 2 fields to the TS type def, and making them optional.

overlookmotel · 2025-04-06T15:25:05Z

I've rebased on latest main and pushed a commit to fix the raw_deser impl. If you could fix the TS type def, I think we'll be good to go.

therewillbecode · 2025-04-08T15:48:58Z

I've rebased on latest main and pushed a commit to fix the raw_deser impl. If you could fix the TS type def, I think we'll be good to go.

@overlookmotel

I have implemented the feedback, feel free to check it over.

overlookmotel

Thanks!

…#10316) Follow-on after #10208. We can remove the `JSXOpeningFragmentAttributes` serializer, and use `TsEmptyArray` instead. This results in `attributes?: []` in the TS type def, which seems reasonable since a JSX fragment can't have attributes. Really this field shouldn't exist at all! It can probably be considered a mistake in `acorn-jsx` that it does.

@camchenry

commit e419aaf Author: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Wed Apr 9 09:54:18 2025 +0000 refactor(ast_tools): move limit for inlining to a const in `Visit` generator (#10291) Pure refactor. `Visit` generator adds `#[inline]` attr to visitor methods containing 5 or less statements. Make this limit a `const`, rather than having it inline in multiple places. commit a605247 Author: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Wed Apr 9 09:54:17 2025 +0000 refactor(ast_tools): make visit call generator functions generic (#10290) Pure refactor. Make functions in `Visit` generator which generate `visitor.visit_*(...)` statements generic over `VisitorOutputs`. `VisitorOutputs` trait represents a set of outputs. In generator for `Visit` and `VisitMut`, there are 2 outputs for the 2 traits. But other generators need to be able to just generate `Visit` alone. Preparatory work for #10292. commit 411610f Author: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Wed Apr 9 09:54:17 2025 +0000 refactor(ast_tools): change methods to free functions in `Visit` generator (#10289) Pure refactor. Move methods of `VisitBuilder` which generate `visitor.visit_*(...)` statements to be free functions, so that they can be exported and used by other generators. Preparatory work for #10292. commit 267922b Author: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Wed Apr 9 09:16:59 2025 +0000 perf(transformer/jsx): speed up decoding `JSXText` strings (#9741) close: #10249 Speed up transforming `JSXText` nodes by: 1. Avoiding allocations (construct strings directly in arena, with no intermediate `String`s). 2. Where no HTML entity decoding or string concatenation is required, reuse a slice of source text, rather than generating a new `Atom` and copying string data. commit 11647e8 Author: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Wed Apr 9 09:16:59 2025 +0000 test(transformer/jsx): tests for `JSXText` strings (#10324) Add tests for transforming `JSXText` and `JSXAttributeValue`. Tests cover: * Whitespace and line breaks. * HTML entities ([`HTMLCharacterReference`](https://facebook.github.io/jsx/#sec-HTMLCharacterReference)). commit b54fb3e Author: Yuji Sugiura <6259812+leaysgur@users.noreply.github.com> Date: Wed Apr 9 17:13:24 2025 +0900 fix(estree): Rename `TSInstantiationExpression`.`type_parameters` to `type_arguments` (#10327) Part of #9705 commit 9734152 Author: Yuji Sugiura <6259812+leaysgur@users.noreply.github.com> Date: Wed Apr 9 16:14:25 2025 +0900 fix(ast): Handle `TSThisType` in `TSTypePredicate` (#10328) Part of #9705 When given this code: ```ts interface X { y(): this is { z: 1 } } ``` Currently the parser treats `this` inside the type annotation as `Identifier` named `this`, but it should be `TSThisType`. commit 81867c4 Author: camc314 <18101008+camc314@users.noreply.github.com> Date: Wed Apr 9 04:39:13 2025 +0000 fix(linter): fix stack overflow in react/exhaustive deps (#10322) fixes #10319 commit a95ba40 Author: Sysix <3897725+Sysix@users.noreply.github.com> Date: Tue Apr 8 22:33:17 2025 +0000 refactor(language_server): make server more error resistance by falling back to default config (#10257) When the client does provide us with an invalid config path / file, the server should not crash. Related oxc-project/oxc-zed#10 #10123 commit e0b6c8c Author: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Tue Apr 8 22:05:34 2025 +0000 fix(transformer/react): correct comment (#10323) Fix comment in function for decoding HTML entities in `JSXText` elements. Hex code escape is `ሴ` not `&x1234;`. Also reformat comments in this function. commit 294d24b Author: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Tue Apr 8 16:15:13 2025 +0000 refactor(ast/estree): simplify serialization for `JSXOpeningFragment` (#10316) Follow-on after #10208. We can remove the `JSXOpeningFragmentAttributes` serializer, and use `TsEmptyArray` instead. This results in `attributes?: []` in the TS type def, which seems reasonable since a JSX fragment can't have attributes. Really this field shouldn't exist at all! It can probably be considered a mistake in `acorn-jsx` that it does. commit cc07efd Author: therewillbecode <tomw08@gmail.com> Date: Tue Apr 8 16:55:36 2025 +0100 fix(ast/estree): Fix `JSXOpeningFragment` (#10208) Fix `JSXOpeningFragment` node estree TS serialization by skipping `selfClosing` and `attributes` fields when AST is TS. Part of our broader work to align our AST's ESTree output with that of TS-ESLint's. Relates to #9705 --------- Co-authored-by: overlookmotel <theoverlookmotel@gmail.com> commit 48ed6a1 Author: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Tue Apr 8 13:07:27 2025 +0000 fix(ast/estree): fix span for `TemplateElement` in TS AST (#10315) Part of #9705. TS-ESLint differs from Acorn in the `span` of `TemplateElement`. TS-ESLint includes the preceding `` ` `` or `}` and following `${` or `` ` `` in the span. ```js const template = `abc${x}def${x}ghi`; // Acorn: ^^^ ^^^ ^^^ // TS-ESLint: ^^^^^^ ^^^^^^ ^^^^^ ``` Make the span follow TS-ESLint in the TS AST. commit 48c711a Author: 翠 / green <green@sapphi.red> Date: Tue Apr 8 19:53:14 2025 +0900 fix(minifier): panic when compressing `a ? b() : b()` (#10311) found in vitejs/rolldown-vite#104 commit 4268b23 Author: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Date: Tue Apr 8 17:13:17 2025 +0800 chore(deps): lock file maintenance rust crates (#10300) commit eba5fcf Author: oxc-bot <boshen_chen@qq.com> Date: Tue Apr 8 13:52:15 2025 +0800 release(crates): v0.63.0 (#10309) commit 52ea978 Author: Ulrich Stark <8657779+ulrichstark@users.noreply.github.com> Date: Tue Apr 8 06:07:48 2025 +0200 refactor(linter): update comments, improve tests, add variant All to LintFilterKind (#10259) I split this PR into easily reviewable commits. While working on this PR, I noticed two possible follow up changes: `LintFilterKind::Rule` and `LintFilterKind::Generic` behave exactly the same in `ConfigStoreBuilder::with_filter` because `LintFilterKind::Rule` never actually uses or checks for the plugin it stores. Should I include the check for plugin? The proposed fifth `LintFilterKind` isn't implemented yet (see: `// TODO: plugin + category? e.g -A react:correctness)`). Should I add it? commit 9aaba69 Author: Sub <9058689+zubhav@users.noreply.github.com> Date: Tue Apr 8 08:07:31 2025 +0400 fix(linter): nested configuration directory resolution (#10157) Fixes #10156 I'm not very familiar with the inner workings of this project - this was a fix based on what I could understand after exploring the codebase for the first time. @camchenry It would be great to get your eyes on this since you are familiar with the nested config implementation. commit e0057c3 Author: Sysix <3897725+Sysix@users.noreply.github.com> Date: Tue Apr 8 01:49:06 2025 +0000 perf(language_server): only restart internal linter once when multiple config changes detected (#10256)

github-actions bot added A-ast Area - AST C-bug Category - Bug labels Apr 3, 2025

therewillbecode force-pushed the fix/jsx-opening-fragment branch from a809be2 to 2c0b300 Compare April 5, 2025 20:42

overlookmotel force-pushed the fix/jsx-opening-fragment branch from 2c0b300 to 6cc3e34 Compare April 6, 2025 15:24

therewillbecode force-pushed the fix/jsx-opening-fragment branch from 70a536f to d3b6f56 Compare April 8, 2025 15:08

therewillbecode and others added 7 commits April 8, 2025 16:18

Prevent serialization of some fields for ts estree

7ce84f2

Remove blank line

3ab9381

Fix deser which failed ci

46995fc

fix raw_deser

44887fc

add comment

3124ca0

Implement PR feedback

1561367

Update snapshot for ts estree

4f7958a

therewillbecode force-pushed the fix/jsx-opening-fragment branch from d3b6f56 to 4f7958a Compare April 8, 2025 15:26

overlookmotel approved these changes Apr 8, 2025

View reviewed changes

overlookmotel merged commit cc07efd into oxc-project:main Apr 8, 2025
25 checks passed

This was referenced Apr 8, 2025

refactor(ast/estree): simplify serialization for JSXOpeningFragment #10316

Merged

ast: #[estree(add_fields(...))] for JSXOpeningFragment should not be applied to TS-ESLint AST #10002

Closed

therewillbecode deleted the fix/jsx-opening-fragment branch April 11, 2025 15:44

oxc-bot mentioned this pull request Apr 17, 2025

release(crates): v0.64.0 #10459

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(ast/estree): Fix `JSXOpeningFragment`#10208

fix(ast/estree): Fix `JSXOpeningFragment`#10208
overlookmotel merged 7 commits intooxc-project:mainfrom
therewillbecode:fix/jsx-opening-fragment

therewillbecode commented Apr 3, 2025

Uh oh!

graphite-app bot commented Apr 3, 2025

Uh oh!

codspeed-hq bot commented Apr 3, 2025 •

edited

Loading

Uh oh!

overlookmotel commented Apr 4, 2025

Uh oh!

overlookmotel commented Apr 4, 2025 •

edited

Loading

Uh oh!

overlookmotel commented Apr 6, 2025

Uh oh!

therewillbecode commented Apr 8, 2025

Uh oh!

overlookmotel left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

therewillbecode commented Apr 3, 2025

Uh oh!

graphite-app bot commented Apr 3, 2025

How to use the Graphite Merge Queue

Uh oh!

codspeed-hq bot commented Apr 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodSpeed Instrumentation Performance Report

Merging #10208 will not alter performance

Summary

Uh oh!

overlookmotel commented Apr 4, 2025

Uh oh!

overlookmotel commented Apr 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

overlookmotel commented Apr 6, 2025

Uh oh!

therewillbecode commented Apr 8, 2025

Uh oh!

overlookmotel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codspeed-hq bot commented Apr 3, 2025 •

edited

Loading

overlookmotel commented Apr 4, 2025 •

edited

Loading