Repository navigation
Represent JSX component annotations with a parsetree node - #8884
Merged
Merged
Conversation
`@react.component`, `@jsx.component` and their `componentWithProps` forms
on a `let`, an `external` or a signature item become `pvb_jsx_component` /
`pval_jsx_component`: the kind (labelled arguments, optionally sharing a
props type given as `(: t)`, or a props record), the spelling, and the
attribute's location. `Ast_helper.Vb.mk` and `Val.mk` take the annotation
out of the attributes, as `Type.field` does for `@as`, and
`Vb.binding_attributes` / `Val.description_attributes` put it back where it
was written for the printers, the mappers and iterators, the v0 PPX bridge
and the typed tree. The JSX transform reads the node instead of scanning
attribute names. An annotation whose payload the transform does not
understand, such as `@react.component("x")`, stays an attribute and is now
reported; it used to be accepted with the payload ignored.
Example (rescript):
module C = {
@react.component("Button")
let make = (~label) => React.string(label)
}
Diagnostic (text):
@react.component takes no payload, or the props type to share, as in
@react.component(: props).
Review: Why a field on the binding, not a structure item of its own?
The annotation is written on a `let`, an `external` or a signature `let`,
and the JSX transform expands it in place. A field keeps that shape, and
bindings the transform leaves alone (local `let`s, `componentWithProps` on
a signature item, everything when JSX is off) keep their meaning without
special cases.
Review: Why does the typed tree get the attribute back?
Nothing downstream reads the node, but `.cmi` attributes and tools that
read typed attributes saw `@react.component` wherever the JSX transform
did not consume it. Restoring the written attributes keeps that, and with
it diagnostics that print those declarations.
Review: Where does the attribute go on the v0 wire?
At its source position among the other attributes. PPX output carries no
positions, so everything ties; the annotation then goes first, where it is
usually written. A PPX that writes it after another attribute sees the
order swapped after a round trip; `@as` has the same limitation.
Review: Do duplicates behave as before?
Yes. The first `@react.component`, or failing that the first
`componentWithProps`, becomes the node, the precedence the transform
always applied. The rest stay attributes: bindings drop them as before,
externals and signature items count them for the existing "Only one JSX
component call" and `componentWithProps` external errors.
Area: syntax printer frontend typechecker analysis tests
Output: unchanged, the test suites' generated JS is byte-identical; 232
generated programs compile to identical JS without JSX, and with JSX 4
except the 45 with an unsupported payload, which now fail with the error
above
Formatting: unchanged, base and branch res_parser print 232 generated and
comment-heavy files identically, plain and through the v0 AST
Formats: ast cmi cmt
PPX: unchanged wire: PPXs see the attributes at their source positions;
ast-mapping fixture JsxComponents.res
Editor: unchanged: iterators see the annotation as an attribute, and
Utils.is_jsx_component reads the node
Errors: none, because the new messages are inline `raise_errorf` calls,
which the catalog's scope leaves out
Tests: +tests/syntax_tests/data/ast-mapping/JsxComponents.res
+tests/build_tests/super_errors/fixtures/react_component_invalid_payload.res
+tests/build_tests/super_errors/fixtures/react_componentWithProps_invalid_payload.res
~tests/ounit_tests/ounit_ast_mapper0_tests.ml
Measured: differential harness, base vs branch bsc -bs-jsx 4: 187 of 232
programs identical, 45 differ by the new payload error; without JSX: 232
of 232 identical
PR: #8884
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014hcyfimX2ss8qj5MZwdMdP
Signed-off-by: Christoph Knittel <ck@cca.io>
cknitt
force-pushed
the
jsx-component-node
branch
from
October 17, 2026 07:58
e5a1dfd to
4f4d9d7
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #8884 +/- ##
==========================================
+ Coverage 81.01% 81.04% +0.02%
==========================================
Files 454 455 +1
Lines 63187 63502 +315
==========================================
+ Hits 51193 51463 +270
- Misses 11994 12039 +45
🚀 New features to boost your workflow:
|
rescript
@rescript/belt
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
|
Developer playground preview: https://rescript-lang.github.io/rescript/dev-playground/?version=pr-8884 |
A definition with two component annotations, such as
`@react.component @react.componentWithProps let make = ...`, is now an
error. On `let` bindings the second annotation used to be dropped silently;
on externals and signature items it was already an error, with messages
that depended on the combination. With no annotation left over after the
node takes one, the JSX transform no longer filters attributes or counts
annotations: externals and signature items match on the node directly.
Example (rescript):
module C = {
@react.component @react.componentWithProps
let make = (~label) => React.string(label)
}
Diagnostic (text):
A definition can have only one JSX component annotation.
Breaking: a `let` with two component annotations stops compiling; delete
the extra one. The one that took effect is the first `@react.component`,
or failing that the first `componentWithProps`.
Review: Which diagnostics change on externals and signature items?
"Only one JSX component call can exist on a component at one time", and
the `componentWithProps` external error where `@react.component` was also
present, become the new message, located at the second annotation.
Area: syntax tests
Output: unchanged, except that the 45 generated programs with two
annotations now fail with the error above; 17 of them compiled before
Formatting: unchanged, base and branch res_parser print 232 generated files
identically
Formats: none
PPX: unchanged
Editor: unchanged
Errors: none, because the message is an inline `raise_errorf` call, which
the catalog's scope leaves out
Tests: +tests/build_tests/super_errors/fixtures/react_component_duplicate_annotation.res
~tests/syntax_tests/data/ast-mapping/JsxComponents.res
Measured: differential harness, base vs branch bsc -bs-jsx 4: 142 of 232
programs identical, 45 differ by the payload error, 45 by this one;
without JSX: 232 of 232 identical
PR: #8884
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014hcyfimX2ss8qj5MZwdMdP
Signed-off-by: Christoph Knittel <ck@cca.io>
cknitt
marked this pull request as ready for review
October 17, 2026 09:47
Member
Author
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
cristianoc
approved these changes
Oct 17, 2026
This was referenced Oct 18, 2026
cknitt
added a commit
that referenced
this pull request
Oct 21, 2026
…ing (#8944) #8884 made a payload on @react.componentWithProps an error, breaking code that compiled before; the payload was always ignored. Claude-Session: https://claude.ai/code/session_01GPwM4m3vhYgbdzgXvmQ7Te Signed-off-by: Christoph Knittel <ck@cca.io> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or 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
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.
Part of #8624 (attribute → node: JSX component attributes).
@react.component,@jsx.componentand theircomponentWithPropsforms become a node on value bindings and value descriptions (pvb_jsx_component/pval_jsx_component), following the recipe used for@asin #8619:Ast_helper.Vb.mk/Val.mktake the annotation out of the attributes.Vb.binding_attributes/Val.description_attributesput it back where it was written for the printers, mappers/iterators, the v0 PPX bridge and the typed tree.The JSX transform reads the node instead of scanning attribute names. The first
@react.component, else the firstcomponentWithProps, becomes the node, the precedence the transform always used.Any component annotation left among the attributes is now an error, which is the only user-facing change:
@react.component("x"),@react.componentWithProps(: t)) used to be accepted, with the payload silently ignored;letbindings, and rejected with assorted messages on externals and signature items.Keeping these as attributes is what lets the formatter preserve them. With no leftovers possible, the transform no longer filters attributes or counts annotations (second commit).
Magic numbers bumped: ast, cmi, cmt.
Verification
I compared base and branch
bscandres_parseron 232 generated programs covering every annotation set × position (module/file/locallet,let … and, external, signaturelet/external, module type), including comments and doc comments between attributes:-bs-jsx 4: identical JS and diagnostics except for the 45 programs with an unsupported payload and the 45 with two annotations (17 of which compiled before), which now hit the new errors;-test-ast-conversion: identical and idempotent.New tests
tests/syntax_tests/data/ast-mapping/JsxComponents.res: the transform still sees every annotation after the v0 round trip. The output is identical to base.ounit_ast_mapper0_tests.ml): extraction, precedence, unknown payloads, v0 attribute order (source positions and PPX ties), spelling, mapper round trip, value descriptions.react_component_invalid_payload.res,react_componentWithProps_invalid_payload.res,react_component_duplicate_annotation.res.🤖 Generated with Claude Code
https://claude.ai/code/session_014hcyfimX2ss8qj5MZwdMdP