tree-sitter-javascript
tree-sitter-javascript copied to clipboard
Various fixes for JSX support
I made some changes to JSX parsing:
- Removed the special treatment of JSX fragments in the grammar, instead the name and attributes of
jsx_opening_elementand the name ofjsx_closing_elementare now optional. This decreases the state count from 1584 to 1533 and simplifies highlighting rules and similar by not requiring special cases for JSX fragments. - Trim whitespace before and after, and split at new lines in
jsx_text(see #221) - Made
</and/>single tokens instead of two, makes more sense to me. - Made the
Foo.Barpart in<Foo.Bar/>have the same parse tree as inconst Component = Foo.Barusing aliases. We get the parse tree(member_expression (identifier) (property_identifier))instead of(nested_identifier (identifier) (identifier)) - Highlight JSX brackets as brackets
- Only highlight JSX tags as tags if they don’t contain periods and start with a lowercase letter (because
<div/>and<Div/>are fundamentally different, the first one refers to the HTML element named "div" while the second one refers to the componentDivwhich is a defined identifier in scope)
I have a corresponding change for the Typescript grammar, but given the way the dependency to the Javascript grammar is constructed, I think I first need to wait for this pull request to be merged before updating the Typescript pull request.
Checklist:
- [X] All tests pass in CI.
- [X] The script/parse-examples passes without issues.
- [X] There are sufficient tests for the new fix/feature.
- [X] Grammar rules have not been renamed unless absolutely necessary.
- [X] The conflicts section hasn't grown too much.
- [X] The parser size hasn't grown too much (check the value of STATE_COUNT in src/parser.c).
This looks good to me. Sorry for the delay.
There seems to be a setup issue for the Windows CI. See https://github.com/tree-sitter/tree-sitter-javascript/runs/7757868479?check_suite_focus=true @dcreager @patrickt @maxbrunsfeld
D:\a\tree-sitter-javascript\tree-sitter-javascript>if not defined npm_config_node_gyp (node "C:\hostedtoolcache\windows\node\14.20.0\x64\node_modules\npm\node_modules\npm-lifecycle\node-gyp-bin\\..\..\node_modules\node-gyp\bin\node-gyp.js" rebuild ) else (node "C:\hostedtoolcache\windows\node\14.20.0\x64\node_modules\npm\node_modules\node-gyp\bin\node-gyp.js" rebuild )
gyp ERR! find VS
gyp ERR! find VS msvs_version not set from command line or npm config
gyp ERR! find VS VCINSTALLDIR not set, not running in VS Command Prompt
gyp ERR! find VS unknown version "undefined" found at "C:\Program Files\Microsoft Visual Studio\2022\Enterprise"
gyp ERR! find VS could not find a version of Visual Studio 2017 or newer to use
gyp ERR! find VS looking for Visual Studio 20[15](https://github.com/tree-sitter/tree-sitter-javascript/runs/7757868479?check_suite_focus=true#step:4:16)
gyp ERR! find VS - not found
gyp ERR! find VS not looking for VS2013 as it is only supported up to Node.js 8
gyp ERR! find VS
gyp ERR! find VS **************************************************************
gyp ERR! find VS You need to install the latest version of Visual Studio
gyp ERR! find VS including the "Desktop development with C++" workload.
gyp ERR! find VS For more information consult the documentation at:
gyp ERR! find VS https://github.com/nodejs/node-gyp#on-windows
gyp ERR! find VS **************************************************************
gyp ERR! find VS
gyp ERR! configure error
gyp ERR! stack Error: Could not find any Visual Studio installation to use
...
I'm not super convinced by the member_expression change. You can't have any member_expression I think inside a JSX element; it can only be a set of identifiers. You can't have (<complex_expr).Bar, which is also a member_expression, so I'd rather keep things more precise.
Otherwise looks good to me.
Just to be clear, the member_expression change does not allow an arbitrary member_expression inside JSX, it still only allows nested identifiers as before. The only difference is that they will be aliased to member_expression so that highlighting rules for member_expressions apply there as well. I think it's reasonable for Foo.Bar to always be highlighted in the same way whether it's in JSX or in an expression.
Can you rebase on the latest to resolve the conflicts?
Ok, I believe I have now resolved the conflicts.
will this be merged anytime soon?
Would love for this to be merged
Can you rebase @guillaumebrunerie (last time :grin:)
@amaanq I have now rebased my branch, regenerated the files using the latest versions of tree-sitter and emcc, and squashed the commits.
Great, thanks