Skip to content

Commit e5a1dfd

Browse files
cknittclaude
andcommitted
Represent JSX component annotations with a parsetree node
`@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 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>
1 parent 4e11b25 commit e5a1dfd

28 files changed

Lines changed: 625 additions & 72 deletions

‎analysis/src/dump_ast.ml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,7 @@ let print_value_binding value ~pos ~indentation =
367367
^ ". "
368368
^ print_core_type pvc_type ~pos
369369
in
370-
print_attributes value.Parsetree.pvb_attributes
370+
print_attributes (Ast_helper.Vb.binding_attributes value)
371371
^ "value" ^ ":\n"
372372
^ add_indentation (indentation + 1)
373373
^ (value.pvb_pat |> print_pattern ~pos ~indentation:(indentation + 1))

‎analysis/src/utils.ml‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -161,10 +161,9 @@ let rec unwrap_if_option (t : Types.type_expr) =
161161
| _ -> t
162162

163163
let is_jsx_component (vb : Parsetree.value_binding) =
164-
vb.pvb_attributes
165-
|> List.exists (function
166-
| {Location.txt = "react.component" | "jsx.component"}, _payload -> true
167-
| _ -> false)
164+
match vb.pvb_jsx_component with
165+
| Some {pjc_kind = Pjc_labelled _} -> true
166+
| Some {pjc_kind = Pjc_with_props} | None -> false
168167

169168
let check_name name ~prefix ~exact =
170169
if exact then name = prefix else starts_with name prefix
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- Represent `@react.component`, `@jsx.component` and their `componentWithProps` forms with a dedicated parsetree node. https://github.com/rescript-lang/rescript/pull/XXXX
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- Report a JSX component annotation with an unsupported payload, such as `@react.component("x")`, instead of ignoring the payload. https://github.com/rescript-lang/rescript/pull/XXXX

‎compiler/ext/config.ml‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
let cmi_magic_number = "Caml1999I037"
1+
let cmi_magic_number = "Caml1999I038"
22

33
(* Magic numbers for marshaled values of the *current* parsetree, whose layout
44
changes across compiler versions. *)
5-
and ast_impl_magic_number = "ResImpl01316"
5+
and ast_impl_magic_number = "ResImpl01317"
66

7-
and ast_intf_magic_number = "ResIntf01316"
7+
and ast_intf_magic_number = "ResIntf01317"
88

99
(* Magic numbers of the frozen Parsetree0 (OCaml 4.06) layout used on the
1010
external-PPX wire. They must never be written in front of a
@@ -13,7 +13,7 @@ and ast0_impl_magic_number = "Caml1999M022"
1313

1414
and ast0_intf_magic_number = "Caml1999N022"
1515

16-
and cmt_magic_number = "Caml1999T044"
16+
and cmt_magic_number = "Caml1999T045"
1717

1818
let get_load_path () = (Compiler_request_state.current ()).load_path
1919
let set_load_path path = (Compiler_request_state.current ()).load_path <- path

‎compiler/frontend/ast_external_mk.ml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ let local_external_apply loc ?(pval_attributes = [])
3636
pval_type;
3737
pval_loc = loc;
3838
pval_prim = Some pval_prim;
39+
pval_jsx_component = None;
3940
pval_attributes;
4041
};
4142
],

‎compiler/frontend/ast_tuple_pattern_flatten.ml‎

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,18 @@ let flattern_tuple_pattern_vb (self : Ast_mapper.mapper)
6060
in
6161
let pvb_expr = self.expr self vb.pvb_expr in
6262
let pvb_attributes = self.attributes self vb.pvb_attributes in
63+
let pvb_jsx_component = vb.pvb_jsx_component in
6364
let pvb_expr_desc = (Ast_payload.unwrap_braces pvb_expr).pexp_desc in
6465
match (pvb_constraint, pvb_pat.ppat_desc, pvb_expr_desc) with
6566
| Some _, _, _ ->
66-
{pvb_pat; pvb_expr; pvb_constraint; pvb_loc = vb.pvb_loc; pvb_attributes}
67+
{
68+
pvb_pat;
69+
pvb_expr;
70+
pvb_constraint;
71+
pvb_jsx_component;
72+
pvb_loc = vb.pvb_loc;
73+
pvb_attributes;
74+
}
6775
:: acc
6876
| None, Ppat_tuple xs, _ when List.for_all is_simple_pattern xs -> (
6977
match Ast_open_cxt.destruct_open_tuple pvb_expr [] with
@@ -77,12 +85,20 @@ let flattern_tuple_pattern_vb (self : Ast_mapper.mapper)
7785
pvb_pat = pat;
7886
pvb_expr = Ast_open_cxt.restore_exp exp wholes;
7987
pvb_constraint = None;
88+
pvb_jsx_component;
8089
pvb_attributes;
8190
pvb_loc = vb.pvb_loc;
8291
}
8392
:: acc)
8493
| _ ->
85-
{pvb_pat; pvb_expr; pvb_constraint; pvb_loc = vb.pvb_loc; pvb_attributes}
94+
{
95+
pvb_pat;
96+
pvb_expr;
97+
pvb_constraint;
98+
pvb_jsx_component;
99+
pvb_loc = vb.pvb_loc;
100+
pvb_attributes;
101+
}
86102
:: acc)
87103
| None, Ppat_record (_, _, Some rest), Pexp_pack {pmod_desc = Pmod_ident _} ->
88104
Location.raise_errorf ~loc:rest.rest_loc
@@ -99,14 +115,22 @@ let flattern_tuple_pattern_vb (self : Ast_mapper.mapper)
99115
Ast_helper.Exp.ident ~loc:lid.loc
100116
{lid with txt = Ldot (id.txt, s)};
101117
pvb_constraint = None;
118+
pvb_jsx_component = None;
102119
pvb_attributes = [];
103120
pvb_loc = pat.ppat_loc;
104121
}
105122
| _ ->
106123
Location.raise_errorf ~loc:lid.loc
107124
"Not supported pattern match on modules")
108125
| _ ->
109-
{pvb_pat; pvb_expr; pvb_constraint; pvb_loc = vb.pvb_loc; pvb_attributes}
126+
{
127+
pvb_pat;
128+
pvb_expr;
129+
pvb_constraint;
130+
pvb_jsx_component;
131+
pvb_loc = vb.pvb_loc;
132+
pvb_attributes;
133+
}
110134
:: acc
111135

112136
let value_bindings_mapper (self : Ast_mapper.mapper)

‎compiler/frontend/bs_builtin_ppx.ml‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -493,6 +493,7 @@ let signature_item_mapper (self : mapper) (sigi : Parsetree.signature_item) :
493493
{
494494
value_desc with
495495
pval_prim = Some (Ast_external_mk.inline_string semantic);
496+
pval_jsx_component = None;
496497
pval_attributes = [];
497498
};
498499
}
@@ -506,6 +507,7 @@ let signature_item_mapper (self : mapper) (sigi : Parsetree.signature_item) :
506507
{
507508
value_desc with
508509
pval_prim = Some (Ast_external_mk.inline_int s);
510+
pval_jsx_component = None;
509511
pval_attributes = [];
510512
};
511513
}
@@ -518,6 +520,7 @@ let signature_item_mapper (self : mapper) (sigi : Parsetree.signature_item) :
518520
{
519521
value_desc with
520522
pval_prim = Some (Ast_external_mk.inline_bigint s);
523+
pval_jsx_component = None;
521524
pval_attributes = [];
522525
};
523526
}
@@ -530,6 +533,7 @@ let signature_item_mapper (self : mapper) (sigi : Parsetree.signature_item) :
530533
{
531534
value_desc with
532535
pval_prim = Some (Ast_external_mk.inline_float s);
536+
pval_jsx_component = None;
533537
pval_attributes = [];
534538
};
535539
}
@@ -543,6 +547,7 @@ let signature_item_mapper (self : mapper) (sigi : Parsetree.signature_item) :
543547
{
544548
value_desc with
545549
pval_prim = Some (Ast_external_mk.inline_bool (txt = "true"));
550+
pval_jsx_component = None;
546551
pval_attributes = [];
547552
};
548553
}
@@ -586,6 +591,7 @@ let structure_item_mapper (self : mapper) (str : Parsetree.structure_item) :
586591
pvb_pat = {ppat_desc = Ppat_var pval_name} as pvb_pat;
587592
pvb_expr;
588593
pvb_constraint = None;
594+
pvb_jsx_component;
589595
pvb_attributes;
590596
pvb_loc;
591597
};
@@ -615,6 +621,7 @@ let structure_item_mapper (self : mapper) (str : Parsetree.structure_item) :
615621
pval_name;
616622
pval_type = Ast_literal.type_string ();
617623
pval_loc = pvb_loc;
624+
pval_jsx_component = None;
618625
pval_attributes = [];
619626
pval_prim = Some (Ast_external_mk.inline_string semantic);
620627
};
@@ -630,6 +637,7 @@ let structure_item_mapper (self : mapper) (str : Parsetree.structure_item) :
630637
pval_name;
631638
pval_type = Ast_literal.type_int ();
632639
pval_loc = pvb_loc;
640+
pval_jsx_component = None;
633641
pval_attributes = [];
634642
pval_prim = Some (Ast_external_mk.inline_int s);
635643
};
@@ -644,6 +652,7 @@ let structure_item_mapper (self : mapper) (str : Parsetree.structure_item) :
644652
pval_name;
645653
pval_type = Ast_literal.type_float;
646654
pval_loc = pvb_loc;
655+
pval_jsx_component = None;
647656
pval_attributes = [];
648657
pval_prim = Some (Ast_external_mk.inline_float s);
649658
};
@@ -660,6 +669,7 @@ let structure_item_mapper (self : mapper) (str : Parsetree.structure_item) :
660669
pval_name;
661670
pval_type = Ast_literal.type_bool ();
662671
pval_loc = pvb_loc;
672+
pval_jsx_component = None;
663673
pval_attributes = [];
664674
pval_prim = Some (Ast_external_mk.inline_bool (txt = "true"));
665675
};
@@ -675,6 +685,7 @@ let structure_item_mapper (self : mapper) (str : Parsetree.structure_item) :
675685
pvb_pat;
676686
pvb_expr;
677687
pvb_constraint = None;
688+
pvb_jsx_component;
678689
pvb_attributes;
679690
pvb_loc;
680691
};

‎compiler/ml/ast_helper.ml‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,15 +321,96 @@ module Str = struct
321321
let attribute ?loc a = mk ?loc (Pstr_attribute a)
322322
end
323323

324+
(* [@react.component] and its variants are taken out of the attributes when a
325+
value binding or value description is built, so every producer - the
326+
parser, a ppx, the frozen AST bridge - agrees on what is a component. *)
327+
module Jsx_component = struct
328+
let of_attribute (({txt; loc}, payload) : attribute) =
329+
let component pjc_kind pjc_namespace =
330+
Some {pjc_kind; pjc_namespace; pjc_loc = loc}
331+
in
332+
match (txt, payload) with
333+
| "react.component", PStr [] -> component (Pjc_labelled None) Pjc_react
334+
| "react.component", PTyp typ ->
335+
component (Pjc_labelled (Some typ)) Pjc_react
336+
| "jsx.component", PStr [] -> component (Pjc_labelled None) Pjc_jsx
337+
| "jsx.component", PTyp typ -> component (Pjc_labelled (Some typ)) Pjc_jsx
338+
| "react.componentWithProps", PStr [] -> component Pjc_with_props Pjc_react
339+
| "jsx.componentWithProps", PStr [] -> component Pjc_with_props Pjc_jsx
340+
| _ -> None
341+
342+
let to_attribute {pjc_kind; pjc_namespace; pjc_loc} : attribute =
343+
let namespace =
344+
match pjc_namespace with
345+
| Pjc_react -> "react"
346+
| Pjc_jsx -> "jsx"
347+
in
348+
match pjc_kind with
349+
| Pjc_labelled None ->
350+
({txt = namespace ^ ".component"; loc = pjc_loc}, PStr [])
351+
| Pjc_labelled (Some typ) ->
352+
({txt = namespace ^ ".component"; loc = pjc_loc}, PTyp typ)
353+
| Pjc_with_props ->
354+
({txt = namespace ^ ".componentWithProps"; loc = pjc_loc}, PStr [])
355+
356+
(* The first [@react.component], or failing that the first
357+
[@react.componentWithProps]: the JSX transform has always given them
358+
this precedence. A later one, or one whose payload is not understood, is
359+
left in the attributes for the JSX transform to deal with, so that
360+
formatting does not drop it. *)
361+
let take attrs =
362+
let take_first is_kind =
363+
let rec go seen = function
364+
| [] -> (None, attrs)
365+
| attr :: rest -> (
366+
match of_attribute attr with
367+
| Some component when is_kind component.pjc_kind ->
368+
(Some component, List.rev_append seen rest)
369+
| _ -> go (attr :: seen) rest)
370+
in
371+
go [] attrs
372+
in
373+
match
374+
take_first (function
375+
| Pjc_labelled _ -> true
376+
| Pjc_with_props -> false)
377+
with
378+
| (Some _, _) as found -> found
379+
| None, _ ->
380+
take_first (function
381+
| Pjc_with_props -> true
382+
| Pjc_labelled _ -> false)
383+
384+
(* Inverse of [take], for printers and the frozen AST bridge. Attributes
385+
print in source order, so the annotation goes back where it was written.
386+
Ppx output has no source position, so all of it ties on [Location.none];
387+
the annotation then goes first among the tied, where it is usually
388+
written. *)
389+
let put_back component attrs =
390+
match component with
391+
| None -> attrs
392+
| Some component ->
393+
let written_earlier (({loc}, _) : attribute) =
394+
loc.loc_start.pos_cnum < component.pjc_loc.loc_start.pos_cnum
395+
in
396+
let earlier, later = List.partition written_earlier attrs in
397+
earlier @ (to_attribute component :: later)
398+
end
399+
324400
module Val = struct
325401
let mk ?(loc = !default_loc) ?(attrs = []) ?prim name typ =
402+
let jsx_component, attrs = Jsx_component.take attrs in
326403
{
327404
pval_name = name;
328405
pval_type = typ;
406+
pval_jsx_component = jsx_component;
329407
pval_attributes = attrs;
330408
pval_loc = loc;
331409
pval_prim = prim;
332410
}
411+
412+
let description_attributes vd =
413+
Jsx_component.put_back vd.pval_jsx_component vd.pval_attributes
333414
end
334415

335416
module Md = struct
@@ -364,13 +445,18 @@ end
364445

365446
module Vb = struct
366447
let mk ?(loc = !default_loc) ?(attrs = []) ?constraint_ pat expr =
448+
let jsx_component, attrs = Jsx_component.take attrs in
367449
{
368450
pvb_pat = pat;
369451
pvb_expr = expr;
370452
pvb_constraint = constraint_;
453+
pvb_jsx_component = jsx_component;
371454
pvb_attributes = attrs;
372455
pvb_loc = loc;
373456
}
457+
458+
let binding_attributes vb =
459+
Jsx_component.put_back vb.pvb_jsx_component vb.pvb_attributes
374460
end
375461

376462
module Type = struct

‎compiler/ml/ast_helper.mli‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,15 @@ module Exp : sig
276276
Location.t -> expression list -> expression option -> expression
277277
end
278278

279+
(** JSX component annotations *)
280+
module Jsx_component : sig
281+
val of_attribute : attribute -> jsx_component option
282+
(** The component an attribute such as [@react.component] denotes, if its
283+
payload is one the JSX transform understands. *)
284+
285+
val to_attribute : jsx_component -> attribute
286+
end
287+
279288
(** Value declarations *)
280289
module Val : sig
281290
val mk :
@@ -285,6 +294,12 @@ module Val : sig
285294
str ->
286295
core_type ->
287296
value_description
297+
(** A [@react.component] annotation in [attrs] is taken out and becomes
298+
[pval_jsx_component]. *)
299+
300+
val description_attributes : value_description -> attrs
301+
(** The attributes as written, with the [@react.component] annotation put
302+
back. For printers, which must reproduce the source. *)
288303
end
289304

290305
(** Type declarations *)
@@ -486,4 +501,10 @@ module Vb : sig
486501
pattern ->
487502
expression ->
488503
value_binding
504+
(** A [@react.component] annotation in [attrs] is taken out and becomes
505+
[pvb_jsx_component]. *)
506+
507+
val binding_attributes : value_binding -> attrs
508+
(** The attributes as written, with the [@react.component] annotation put
509+
back. For printers, which must reproduce the source. *)
489510
end

0 commit comments

Comments
 (0)