Add jsx_tag_name #7760
Add jsx_tag_name #7760
Conversation
rescript
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/win32-x64
commit: |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new jsx_tag_name type to improve JSX tag name handling in the ReScript compiler, replacing the previous Longident.t representation. The change enables better error handling for invalid JSX tag names and provides more precise differentiation between lowercase tags, qualified lowercase tags, and uppercase tags.
- Introduces a new
jsx_tag_nametype with variants for different JSX tag categories - Updates JSX parsing logic to use the new type and provide better error messages
- Refactors printer and analysis code to work with the new type structure
Reviewed Changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| compiler/ml/parsetree.ml | Adds new jsx_tag_name type definition with variants for different tag types |
| compiler/syntax/src/res_core.ml | Replaces JSX parsing logic to use new tag name type |
| compiler/syntax/src/res_printer.ml | Updates JSX printing to handle new tag name structure |
| compiler/ml/ast_helper.ml | Adds utility functions for jsx_tag_name conversion |
| tests/syntax_tests/data/parsing/errors/expressions/jsx.res | Adds test cases for trailing dots in JSX tag names |
| analysis/src/CompletionFrontEnd.ml | Updates completion logic to handle new tag name type |
| jsx_v4.ml | Updates JSX transformation to work with new tag name variants |
| "detail": "module Comp", | ||
| "documentation": null | ||
| }] | ||
| JSX <O.:__ghost__[0:-1->0:-1] > _children:None |
There was a problem hiding this comment.
This tests <0, which is nonsensical to begin with. I don't mind that we lose this single questionable completion here.
| -> ( | ||
| let name = Ast_helper.Jsx.string_of_jsx_tag_name tag_name.txt in | ||
| let tag_loc = tag_name.loc in | ||
| match tag_name.txt with |
There was a problem hiding this comment.
This highlights a good benefit of the AST change.
|
@cristianoc yes, I like it. It makes things a bit more clear. The parser is very conscious about parsing jsx element tags and attributes. And the ppx also benefits from it. |
Following up on the suggestion from @cristianoc.