Simplify JSX and TSX lexers #1492
Merged
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
|
@louisrli In the course of trying to fix the TSX lexer, I ended up writing a version that didn't rely on the JSX lexer at all. I thought it was a significant enough improvement that I essentially replaced the JSX lexer with it. It now appears to lex your buggy case properly:
Let me know if you spot anything amiss! |
pyrmont
changed the title
|
As a note: I'll merge this into master by making separate PRs that are more targeted. This is mainly intended as a proof of concept of how the new architecture would fit together. |
|
Looking at this a few weeks after writing it, I don't think it is a problem to merge this as a single commit. |
pyrmont
force-pushed
the
bugfix.jsx-simplification
branch
from
fbefe54 to
ebb9ad1
Compare
pyrmont
force-pushed
the
bugfix.jsx-simplification
branch
from
ebb9ad1 to
c375d0d
Compare
pyrmont
changed the title
mattt
pushed a commit
to NSHipster/rouge
that referenced
this pull request
May 19, 2021
The JSX lexer handles interpolation by running an embedded version of itself. This is not the manner in which other lexers handle interpolation and so makes maintenance difficult. It also greatly complicates subclassing the lexer (a practical concern because the TSX lexer subclasses the JSX lexer). To fix this, this commit rewrites the JSX lexer to simplify its structure. In particular, by specifying `:interpol` and `:interpol_inner` states, the need for an embedded version of the lexer is obviated.
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.
The JSX lexer is implemented with an embedded version of itself that is used for lexing interpolated text. This is not the manner in which other lexers handle interpolation and so makes maintenance difficult as patterns developed in other lexers for handling interpolation cannot be brought across. It also makes subclassing the lexer difficult (a practical concern because the TSX lexer subclasses the JSX lexer).
To fix this, this PR rewrites the JSX lexer. By using two interpolation states, it obviates the need for an embedded version.
In the course of implementation, it was discovered that the JavaScript lexer contains a bug in the:statementstate. This state contains a rule for lexing braces but this rule causes the:objectstate to never be closed. The bug was discovered in the implementation of the rewrite because braces are used to begin and end interpolation.Having a simplified implementation of the JSX lexer makes it straightforward to implement a more robust TSX lexer. The PR includes a version of the TSX lexer that fixes a bug where generics that are expressed in the form<T,>would not be lexed correctly.This fixes #1517.