Implement parser changes for the JSX automatic runtime #739
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Progress toward #585 This change extends the parser to detect two JSX cases that are needed for the automatic runtime. The new code is run unconditionally, even in the old transform, since flagging it would have its own overhead and complexity. The new JSX transform needs to distinguish children being static or non-static, where static is equivalent to having at least two comma-separated children emitted by the old transform or any child being a spread child. The main challenge in getting this right is that JSX whitespace-trimming will sometimes completely remove all content from a text range, in which case it shouldn't count as a child for the purposes of counting the number of children. Fortunately, there is a relatively simple algorithm to detect if a text range is empty: it's empty if and only if it is entirely whitespace and has at least one newline. To identify these "empty text" regions, I added a new token type `jsxEmptyText` that is treated the same except for the purposes of counting children. In the future, it likely is reasonable to not treat such a region as a token in the first place. The new JSX transform also needs to detect the pattern of a key appearing after a prop spread. We don't do keyword detection on JSX prop names, so instead I manually detect the name "key", but only if we have already seen a prop spread.
Codecov Report
@@ Coverage Diff @@
## main #739 +/- ##
==========================================
+ Coverage 87.17% 87.23% +0.06%
==========================================
Files 54 54
Lines 5722 5751 +29
Branches 1349 1357 +8
==========================================
+ Hits 4988 5017 +29
Misses 466 466
Partials 268 268
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Benchmark resultsBefore this PR: 397.7 thousand lines per second Measured change: 0.46% faster (0.77% slower to 0.7% faster) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Progress toward #585
This change extends the parser to detect two JSX cases that are needed for the
automatic runtime. The new code is run unconditionally, even in the old
transform, since flagging it would have its own overhead and complexity.
The new JSX transform needs to distinguish children being static or non-static,
where static is equivalent to having at least two comma-separated children
emitted by the old transform or any child being a spread child. The main
challenge in getting this right is that JSX whitespace-trimming will sometimes
completely remove all content from a text range, in which case it shouldn't
count as a child for the purposes of counting the number of children.
Fortunately, there is a relatively simple algorithm to detect if a text range
is empty: it's empty if and only if it is entirely whitespace and has at least
one newline. To identify these "empty text" regions, I added a new token type
jsxEmptyTextthat is treated the same except for the purposes of countingchildren. In the future, it likely is reasonable to not treat such a region as a
token in the first place.
The new JSX transform also needs to detect the pattern of a key appearing after
a prop spread. We don't do keyword detection on JSX prop names, so instead I
manually detect the name "key", but only if we have already seen a prop spread.