Repository navigation
Reject bindings of a module that get the same JavaScript name - #8982
Conversation
Why:
`Ext_ident.convert` maps both `delete` and `$$delete` to `$$delete`,
and `\"a-b"` like `a$b`, so two bindings could share an export name or
a module object key: the ES export list repeated a name, which Node
rejects, and CommonJS output and nested modules lost a binding. The
type checker now rejects such a module, at the second binding. This
replaces Lam_coercion's "is exported twice" check, which only covered
equal source names and had no location.
Diagnostic added (JavaScript name clash):
Example:
let delete = 1
let $$delete = 2
Before (js):
let $$delete = 1;
let $$delete$1 = 2;
export {
$$delete,
$$delete$1 as $$delete,
}
After (diagnostic):
This value has the same JavaScript name $$delete
as the value at A.res:1:5-10.
Rename one of them.
Diagnostic modified (exception and module of one name):
Example:
exception M(int)
module M = {
let x = 3
}
Before (diagnostic): M is exported twice
After (diagnostic):
This module has the same JavaScript name M
as the extension constructor at A.res:1:1-16.
Rename one of them.
Review: Why not rename one of the two exports? Importers derive an export's name from the source name alone (`js_export_name`), so a renamed export would not be found by them; the clash has to be resolved in the source.
Review: Which signatures are checked? Each structure's signature and each written signature (module types and `.resi` files), since every module block is built from one of them. The exports of a compilation unit keep `default` as written, so only nested modules reject `default` beside `$$default`.
Signed-off-by: Christoph Knittel <ck@cca.io>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xk6AeCYpCK4SZRtHYKHGoe
Semantic Diff1 commit · Full page Reject bindings of a module that get the same JavaScript namea781efb · Christoph Knittel with Claude Opus 5.5 · Oct 23, 2026
Diagnostic added (JavaScript name clash) let delete = 1
let $$delete = 2
let $$delete = 1;
let $$delete$1 = 2;
export {
$$delete,
$$delete$1 as $$delete,
}
Diagnostic modified (exception and module of one name) exception M(int)
module M = {
let x = 3
}
-M is exported twice
+This module has the same JavaScript name M
+as the extension constructor at A.res:1:1-16.
+Rename one of them.Review
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #8982 +/- ##
=======================================
Coverage 81.22% 81.23%
=======================================
Files 457 457
Lines 63862 63873 +11
=======================================
+ Hits 51875 51889 +14
+ Misses 11987 11984 -3
🚀 New features to boost your workflow:
|
rescript
@rescript/belt
@rescript/darwin-arm64
@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-8982 |
|
@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. More of your lovely PRs please. 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". |
2 participants
Fixes #8974.
Ext_ident.convertis not injective:deleteand$$deleteboth become$$delete, and an exotic name such as\"a-b"becomesa$b, the same as the plaina$bthat #8791 allows. Since export names and module object keys are that conversion of the source name, two bindings could end up with one JavaScript name:export { $$delete, $$delete$1 as $$delete }, which Node rejects;exports.$$deleteassigned twice, so the first binding is lost;{ $$delete: 3, $$delete: 4 }, soM.deletereads the wrong value.Renaming one of the exports isn't possible, because importers compute an export's name from the source name alone. So the type checker now rejects such a module, at the second binding:
Typemod.check_js_namesruns on each structure's signature (after shadowed values are dropped) and on each written signature: module types, and.resifiles at the top level. Every module block is built from one of these. Values, extension constructors and modules count; externals, types andinternal.localmodules don't. The exports of a compilation unit keepdefaultas written, sodefaultbeside$$defaultis still allowed at the top level and rejected only in nested modules.The new check also covers the case that
Lam_coercionreported as "M is exported twice" (an exception and a module with the same name), so that check and theBs_duplicate_exportserror are removed, along withHash_set_string, which was used only there.gpr_978now expects the new message.The flat
A$B$makeexports of nestedmakevalues are added after type checking and are outside this check. They already have one of their own: a top-levellet \"A$B$make"besideA.B.makeis rejected with "Cannot exportA.B.makeasA$B$makebecause that name is already exported."Tests
super_errorsfixturesjs_name_clash_keyword,js_name_clash_nested(default/$$default),js_name_clash_mangled(\"a-b"/a$b),js_name_clash_module(exception and module) andjs_name_clash_module_type.super_errors_multifixtureJs_name_clash_interfacefor a.resi.🤖 Generated with Claude Code
https://claude.ai/code/session_01Xk6AeCYpCK4SZRtHYKHGoe