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 support for automatic JSX runtime #740

Merged
merged 10 commits into from Sep 12, 2022
Merged

Add support for automatic JSX runtime #740

merged 10 commits into from Sep 12, 2022

Conversation

alangpierce
Copy link
Owner

@alangpierce alangpierce commented Aug 19, 2022

Fixes #585

Tech plan that includes many major details:
https://github.com/alangpierce/sucrase/wiki/JSX-Automatic-Runtime-Transform-Technical-Plan

This PR adds two new options:

  • jsxRuntime, which can be "classic" (the existing behavior) or "automatic"
    (the transform behavior released with React 17).
  • jsxImportSource, which configures the import prefix when using the automatic
    runtime.

This PR also adds docs for the new options and makes them available to demo on the website.

While it may have been possible to split the JSXTransform code into multiple files, the
configuration space is intertwined enough that it seemed better for now to just have a unified
implementation for now. This leads to a little extra abstraction when resolving function names
as well as some special cases about where to place commas, but the overall flow of logic is the
same.

Unfortunately, it looks like the new implementation is slower than the old one, regardless of
configuration, which isn't too surprising given the added complexity. Some measurements,
using a higher-scale variation of yarn benchmark sample (which specifically tests scale for
the JSX transform):

  • Before this PR: 968k lines per second
  • Classic runtime after this PR: 925k lines per second (4% slower than before)
  • Automatic runtime after this PR: 901k lines per second (7% slower than before)

There's certainly some possible room for improvement, e.g. by more aggressively caching/
precomputing resolved names or duplicating and individually simplifying the code path for
each configuration, but that can be done as future work.

Fixes #585

Tech plan:
https://github.com/alangpierce/sucrase/wiki/JSX-Automatic-Runtime-Transform-Technical-Plan

This PR adds two new options:
* `jsxRuntime`, which can be "classic" (the existing behavior) or "automatic"
  (the transform behavior released with React 17).
* `jsxImportSource`, which configures the import prefix when using the automatic
  runtime.

This PR also makes these options available for the website.
@codecov
Copy link

codecov bot commented Aug 19, 2022

Codecov Report

Merging #740 (e351cff) into main (e9bc44f) will increase coverage by 0.19%.
The diff coverage is 97.56%.

@@            Coverage Diff             @@
##             main     #740      +/-   ##
==========================================
+ Coverage   87.23%   87.43%   +0.19%     
==========================================
  Files          54       54              
  Lines        5751     5856     +105     
  Branches     1357     1389      +32     
==========================================
+ Hits         5017     5120     +103     
  Misses        466      466              
- Partials      268      270       +2     
Impacted Files Coverage Δ
src/CJSImportProcessor.ts 91.91% <ø> (ø)
src/Options-gen-types.ts 100.00% <ø> (ø)
src/Options.ts 100.00% <ø> (ø)
src/transformers/JSXTransformer.ts 97.11% <97.38%> (-0.06%) ⬇️
src/TokenProcessor.ts 94.78% <100.00%> (+0.97%) ⬆️
src/parser/plugins/jsx/index.ts 92.35% <100.00%> (+0.09%) ⬆️
src/parser/tokenizer/index.ts 86.31% <100.00%> (+0.02%) ⬆️

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

@github-actions
Copy link

github-actions bot commented Aug 19, 2022

Benchmark results

Before this PR: 414.1 thousand lines per second
After this PR: 410.8 thousand lines per second

Measured change: 0.8% slower (1.34% slower to 0.3% faster)
Summary: Likely no significant difference

alangpierce added a commit that referenced this pull request Sep 2, 2022
This PR has a few details pulled from #740 to make it easier to read the
substantial changes in that PR:
* Reorder the options interface to better group related options together.
* Reorder the methods in JSXTransformer to be in (roughly) top-down topological
  order rather than the more scattered order that it was previously.
* Change website advanced options to dynamically only show the ones relevant
  based on what is selected.
alangpierce added a commit that referenced this pull request Sep 2, 2022
This PR has a few details pulled from #740 to make it easier to read the
substantial changes in that PR:
* Reorder the options interface to better group related options together.
* Reorder the methods in JSXTransformer to be in (roughly) top-down topological
  order rather than the more scattered order that it was previously.
* Change website advanced options to dynamically only show the ones relevant
  based on what is selected.
alangpierce added a commit that referenced this pull request Sep 2, 2022
This PR has a few details pulled from #740 to make it easier to read the
substantial changes in that PR:
* Reorder the options interface to better group related options together.
* Reorder the methods in JSXTransformer to be in (roughly) top-down topological
  order rather than the more scattered order that it was previously.
* Change website advanced options to dynamically only show the ones relevant
  based on what is selected.
alangpierce added a commit that referenced this pull request Sep 2, 2022
This PR has a few details pulled from #740 to make it easier to read the
substantial changes in that PR:
* Reorder the options interface to better group related options together.
* Reorder the methods in JSXTransformer to be in (roughly) top-down topological
  order rather than the more scattered order that it was previously.
* Change website advanced options to dynamically only show the ones relevant
  based on what is selected.
@alangpierce alangpierce merged commit 3e88b06 into main Sep 12, 2022
@alangpierce alangpierce deleted the new-jsx-transform branch September 12, 2022 16:18
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.

Add support for new React 17 JSX transform
1 participant