Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add "preserve" mode for JSX #788

Merged
merged 4 commits into from Mar 26, 2023
Merged

Add "preserve" mode for JSX #788

merged 4 commits into from Mar 26, 2023

Conversation

alangpierce
Copy link
Owner

Fixes #780
Fixes #559

This PR adds support for preserving JSX as-is. Since the parser need to know if JSX is enabled or not, preserving JSX is accomplished by keeping "jsx" as a transform, but setting the jsxRuntime mode to "preserve".

Most of the details just work: the imports and typescript transform are mostly able to remove type syntax and transform imported names properly. The one missing case was handling situations like <div>, which is not actually an identifier access and thus should not be transformed by the imports transform. This required adding a special case to the parser to remove the identifier role in that case.

For now, the ts-node integration does not recognize this option because Node wouldn't be able to recognize JSX anyway.

Fixes #780
Fixes #559

This PR adds support for preserving JSX as-is. Since the parser need to know if
JSX is enabled or not, preserving JSX is accomplished by keeping `"jsx"` as a
transform, but setting the `jsxRuntime` mode to `"preserve"`.

Most of the details just work: the imports and typescript transform are mostly
able to remove type syntax and transform imported names properly. The one
missing case was handling situations like `<div>`, which is not actually an
identifier access and thus should not be transformed by the imports transform.
This required adding a special case to the parser to remove the identifier role
in that case.

For now, the ts-node integration does not recognize this option because Node
wouldn't be able to recognize JSX anyway.
The option only affects information emitted with the other two transform modes.
@codecov
Copy link

codecov bot commented Mar 25, 2023

Codecov Report

Merging #788 (a65e549) into main (365fccc) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #788   +/-   ##
=======================================
  Coverage   88.32%   88.32%           
=======================================
  Files          55       55           
  Lines        5945     5954    +9     
  Branches     1410     1413    +3     
=======================================
+ Hits         5251     5259    +8     
  Misses        430      430           
- Partials      264      265    +1     
Impacted Files Coverage Δ
src/Options-gen-types.ts 100.00% <ø> (ø)
src/Options.ts 100.00% <ø> (ø)
src/transformers/JSXTransformer.ts 97.11% <ø> (ø)
src/parser/plugins/jsx/index.ts 92.69% <100.00%> (+0.34%) ⬆️
src/transformers/RootTransformer.ts 93.68% <100.00%> (+0.03%) ⬆️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@github-actions
Copy link

github-actions bot commented Mar 25, 2023

Benchmark results

Before this PR: 395.7 thousand lines per second
After this PR: 396.6 thousand lines per second

Measured change: 0.21% faster (1.71% slower to 0.76% faster)
Summary: Likely no significant difference

@alangpierce alangpierce merged commit 5636395 into main Mar 26, 2023
8 checks passed
@alangpierce alangpierce deleted the preserve-jsx branch March 26, 2023 03:55
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.

jsx: preserve Preserve jsx
1 participant