Add @res.hoistedFunction support for flat JS export - #8402
Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6dfddc0098
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
rescript
@rescript/belt
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
b0e30dd to
da4b5b3
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da4b5b3307
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #8402 +/- ##
==========================================
+ Coverage 75.95% 75.97% +0.02%
==========================================
Files 474 474
Lines 62905 63074 +169
==========================================
+ Hits 47779 47920 +141
- Misses 15126 15154 +28
🚀 New features to boost your workflow:
|
|
Developer playground preview: https://rescript-lang.github.io/rescript/dev-playground/?version=pr-8402 |
da4b5b3 to
8595ead
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8595ead79d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| match pat.pat_desc with | ||
| | Tpat_var (id, _) -> | ||
| if allow_js_hoist then Ident.make_js_hoisted id | ||
| else | ||
| Location.prerr_warning loc | ||
| (Warnings.Misplaced_attribute hoisted_function_attr_name) | ||
| | _ -> ()) |
There was a problem hiding this comment.
When a hoisted binding has an explicit type annotation, such as @res.hoistedFunction let make: unit => string = () => "ok", type checking represents the variable as Tpat_alias ({pat_desc = Tpat_any}, id, _), not Tpat_var. This fallback therefore silently skips the attribute, producing neither the flat export nor a warning. Handle the alias form already accepted by transl_let for recursive bindings.
Useful? React with 👍 / 👎.
| ( Translcore.transl_let | ||
| ~allow_js_hoist:(not (is_top rootpath)) | ||
| rec_flag pat_expr_list body, |
There was a problem hiding this comment.
For a function annotated inside a local module or a functor body, rootpath is None or a Papply, so this condition enables hoisting even though no fixed top-level export path exists. js_hoisted_aliases later scans only exported top-level module blocks, causing the marker to be silently discarded with neither the promised flat export nor the misplaced-attribute warning. Only enable the attribute when the enclosing path can actually be materialized, or diagnose these contexts.
Useful? React with 👍 / 👎.
Signed-off-by: Christoph Knittel <ck@cca.io>
8595ead to
3c50e90
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c50e90bf7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| let has_exportable_module_path = function | ||
| | Some path -> ( | ||
| match Path.flatten path with | ||
| | `Ok (_, _ :: _) -> true |
There was a problem hiding this comment.
When a .resi or an explicit module signature omits the annotated function (or its enclosing module), this check still permits the annotation because it considers only the syntactic path. Signature coercion subsequently removes the field, and js_hoisted_aliases scans only exported module blocks, so compilation succeeds without producing either the flat export or a misplaced-attribute warning. Validate actual post-coercion visibility or diagnose marked bindings for which no alias was generated.
Useful? React with 👍 / 👎.
| else | ||
| Location.prerr_warning loc | ||
| (Warnings.Misplaced_attribute hoisted_function_attr_name) | ||
| | _ -> ()) |
There was a problem hiding this comment.
For a function RHS bound with another irrefutable pattern, such as @res.hoistedFunction let (f as g) = () => "ok", the typed pattern is an alias around a variable rather than either accepted form. This fallback silently ignores the attribute, emitting neither a flat export nor the misplaced-attribute warning; reject unsupported patterns explicitly instead of treating the attribute as handled.
AGENTS.md reference: AGENTS.md:L43-L43
Useful? React with 👍 / 👎.
Signed-off-by: Christoph Knittel <ck@cca.io>
Signed-off-by: Christoph Knittel <ck@cca.io>
46cebb7 to
ea70870
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea708706b9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| | Some path -> | ||
| Some | ||
| (fun id loc -> | ||
| js_hoisted := (id, path @ [id.Ident.name], loc) :: !js_hoisted) |
There was a problem hiding this comment.
When an exported structure contains @internal.local module Hidden = { @res.hoistedFunction let f = ... }, this handler accepts Hidden.f because it has a syntactically fixed path. However, the Tstr_module branch excludes Hidden from the module fields via Typemod.rescript_hide, and js_hoisted_aliases scans only exported top-level module blocks, so no flat alias or metadata is emitted and compilation produces no misplaced-attribute warning. Account for internal.local visibility when creating the handler, just as signature-hidden enclosing modules are rejected.
Useful? React with 👍 / 👎.
Signed-off-by: Christoph Knittel <ck@cca.io>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb7e60c88e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
Signed-off-by: Christoph Knittel <ck@cca.io>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 24763f159e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| match find_field field 0 names args with | ||
| | Some (pos, arg) -> find_path arg fields ((pos, field) :: positions) |
There was a problem hiding this comment.
When a module shadows a value name—such as a marked let make = first followed by an unmarked let make = second—the recorded hoist contains only the string path, and this lookup selects whichever "make" field occurs first in the optimized module block. The flat export can therefore invoke the shadowing function rather than the binding carrying the attribute; retain the registered Ident.t through lowering or reject marked bindings that are subsequently shadowed.
Useful? React with 👍 / 👎.
| | Tstr_type _ | Tstr_modtype _ | Tstr_open _ | Tstr_attribute _ -> | ||
| transl_structure loc fields cc rootpath final_env rem) |
There was a problem hiding this comment.
When @res.hoistedFunction is placed on a type or module-type declaration, this branch skips the item without inspecting its attributes, so compilation succeeds while silently ignoring a function-only attribute. The new implementation explicitly diagnoses externals and unsupported value bindings, so these declaration contexts should likewise emit Misplaced_attribute instead of accepting a no-op annotation.
Useful? React with 👍 / 👎.
| @res.hoistedFunction | ||
| let make = () => "one" |
There was a problem hiding this comment.
The changed test set adds end-to-end and error fixtures, but no tests/syntax_tests coverage or Lambda-layer unit coverage for the new language feature. Add those missing layers, including verification that the marker survives translation and that invalid placements round-trip or diagnose correctly, as required by the repository's compiler testing policy.
AGENTS.md reference: AGENTS.md:L39-L41
Useful? React with 👍 / 👎.
1 participant
Summary
Adds
@res.hoistedFunction, a compiler-supported attribute for exporting nested module functions through flat JS exports.A function defined inside an exported module can now be marked as hoisted. The compiler keeps the normal nested module shape, but also emits a root-level alias/export for the function and records that alias in
.cmjmetadata so downstream modules can import it directly.Motivation
Nested module functions are normally emitted and consumed through property access:
This is the first step for my take on #8293. That work needs generated JSX code for React Server Components to expose nested component functions through stable flat JS exports:
Because producers and consumers are compiled separately, the consumer cannot infer from the source path alone whether
Producer.A.B.makealso has a flat export. The producer therefore records hoisted paths in.cmjmetadata, and consumers use that metadata to emit the flat import when it is available.Design
Source Marker
The feature introduces:
The attribute currently applies only to function bindings. If it is placed on a non-function value or an external declaration, the compiler reports a misplaced-attribute warning.
The attribute deliberately applies only to nested module function bindings with a fixed exported path, matching the intended use case. Local functions, functions declared directly at the file root, local modules, and functor bodies cannot form such an export and produce a misplaced-attribute warning. The complete path must also remain visible after signature coercion: if an explicit module signature or the file's
.resihides the function or an enclosing module, the compiler reports the same warning and does not generate an alias.Producer Output
When an exported module contains a hoisted nested function, the compiler keeps the original module structure and adds a root-level alias.
For a source path like:
the compiler emits a flat export:
The nested function remains available at its normal path, while the flat alias is exported as a separate JS value.
Path Identity and Exotic Identifiers
The flat JavaScript name cannot safely identify the original source path by itself.
Representing a nested path by joining its segments with
$would be ambiguous because ReScript allows escaped identifiers containing$. For example, these are distinct source paths:However, both have the same natural flat JavaScript spelling:
Encoding the path segments in the generated export name would remove the ambiguity, but it would also produce surprising JavaScript names and prevent exotic identifiers from retaining their intended spelling.
Instead, the implementation separates source-path identity from the generated JavaScript name:
.cmjmetadata stores the exact source path as a list of segments.Consumers therefore look up hoisted exports using the structural source path, while the generated JavaScript remains readable and predictable.
If two hoisted paths produce the same JavaScript export name, or if that name conflicts with an existing top-level binding, compilation fails with a clear error.
.cmjMetadataThe
.cmjformat stores each hoisted export as:For example,
A.B.makeis represented as:{ path: ["A", "B", "make"]; name: "A$B$make"; }The path is used for exact source-level lookup. The name is the compiler identifier used for the flat JavaScript export.
This metadata remains separate from the regular
valuestable, which continues to hold arity and cross-module optimization information.Consumer Lookup
When compiling a cross-module nested read such as:
normal compilation looks up the first field,
A, in the producer’s.cmjand emits the remaining property accesses:For a nested read, the compiler now also reconstructs the exact source path and checks the producer’s hoisted export metadata.
If the path is present, it uses the recorded flat export name:
Otherwise, it falls back to the normal property chain.
Because lookup uses the structural path rather than a
$-joined key, exotic identifiers cannot accidentally resolve to another binding’s hoisted export.Performance
Modules without
@res.hoistedFunctionrequire only a shallow, allocation-free scan of their top-level bindings.For annotated functions, the compiler checks the recorded source path against any applicable signature coercions. This is compile-time only and proportional to the short module path and signature involved.
For each annotated function, the generated JavaScript contains one additional alias assignment and export. Calls use the flat export directly; there is no runtime path lookup or encoding.
Consumer lookup is compile-time only and scans the dependency’s normally very small list of hoisted exports.
Tests
Adds coverage for:
$identifiers;.resifiles;