-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Normative: make re-exporting a namespace object in 2 steps behave like 1 step #3715
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
The rendered spec for this PR is available at https://tc39.es/ecma262/pr/3715. |
Co-authored-by: Nicolò Ribaudo <[email protected]>
…=jonco This patch implements tc39/ecma262#3715, to make import * as ns from "mod"; export { ns }; behave the same as export * as ns from "mod"; Tested by tc39/test262#4606 Differential Revision: https://phabricator.services.mozilla.com/D274013
https://bugs.webkit.org/show_bug.cgi?id=303141 Reviewed by NOBODY (OOPS!). This patch changes JSC's handling of `import * as ns; export { ns }` to align with tc39/ecma262#3715, which reached consensus in the Nov 2025 TC39 meeting. This will be tested by tc39/test262#4606, for now I manually verified that the relevant tests in that PR are failing before this patch and passing after it. * Source/JavaScriptCore/parser/ModuleAnalyzer.cpp: (JSC::ModuleAnalyzer::exportVariable):
…=jonco This patch implements tc39/ecma262#3715, to make import * as ns from "mod"; export { ns }; behave the same as export * as ns from "mod"; Tested by tc39/test262#4606 Differential Revision: https://phabricator.services.mozilla.com/D274013 UltraBlame original commit: d14a908552e6d040c7538351430b3152a4a36887
…=jonco This patch implements tc39/ecma262#3715, to make import * as ns from "mod"; export { ns }; behave the same as export * as ns from "mod"; Tested by tc39/test262#4606 Differential Revision: https://phabricator.services.mozilla.com/D274013 UltraBlame original commit: d14a908552e6d040c7538351430b3152a4a36887
…=jonco This patch implements tc39/ecma262#3715, to make import * as ns from "mod"; export { ns }; behave the same as export * as ns from "mod"; Tested by tc39/test262#4606 Differential Revision: https://phabricator.services.mozilla.com/D274013 UltraBlame original commit: d14a908552e6d040c7538351430b3152a4a36887
https://bugs.webkit.org/show_bug.cgi?id=303141 Reviewed by NOBODY (OOPS!). This patch changes JSC's handling of `import * as ns; export { ns }` to align with tc39/ecma262#3715, which reached consensus in the Nov 2025 TC39 meeting. This will be tested by tc39/test262#4606, for now I manually verified that the relevant tests in that PR are failing before this patch and passing after it. * Source/JavaScriptCore/parser/ModuleAnalyzer.cpp: (JSC::ModuleAnalyzer::exportVariable):
https://bugs.webkit.org/show_bug.cgi?id=303141 Reviewed by Yusuke Suzuki and Sosuke Suzuki. This patch changes JSC's handling of `import * as ns; export { ns }` to align with tc39/ecma262#3715, which reached consensus in the Nov 2025 TC39 meeting. This will be tested by tc39/test262#4606, for now I manually verified that the relevant tests in that PR are failing before this patch and passing after it. * Source/JavaScriptCore/parser/ModuleAnalyzer.cpp: (JSC::ModuleAnalyzer::exportVariable): Canonical link: https://commits.webkit.org/303952@main
|
The change has been implemented in SpiderMonkey and JavaScriptCore. It has not been implemented in V8 yet, but V8 also did not match the original behavior. |
|
Since this is just making an error into a non-error I think that's good enough, no need to wait for them to ship. |
https://bugs.webkit.org/show_bug.cgi?id=303141 Reviewed by Yusuke Suzuki and Sosuke Suzuki. This patch changes JSC's handling of `import * as ns; export { ns }` to align with tc39/ecma262#3715, which reached consensus in the Nov 2025 TC39 meeting. This will be tested by tc39/test262#4606, for now I manually verified that the relevant tests in that PR are failing before this patch and passing after it. * Source/JavaScriptCore/parser/ModuleAnalyzer.cpp: (JSC::ModuleAnalyzer::exportVariable): Canonical link: https://commits.webkit.org/303952@main
|
V8 issue (at least according to test262.status): https://issues.chromium.org/issues/464065358 @michaelficarra wants to hold off for a couple months to see if V8 runs into any difficulties, but if they haven't landed by the time we cut the next yearly edition we'll include this either way. |
|
This is a bigger change on our end since apparently we never treated "export * as x" as proper re-exports. Hence the current behavior where you could not require such a namespace through multiple paths. I think there is a non-zero chance that we will run into problems. |
|
If I understand correctly then this change modifies the behavior of deferred imports. If the re-exported namespace object was imported using a deferred import then the result was a deferred namespace object i.e. you got the deferred namespace object from the re-export. If you replace these 2 steps by a direct re-export then you get a regular namespace object (because there is nothing like a deferred export). Am I missing something or is this expected? |
|
@iamstolis The import defer proposal would be updated to generate a binding that directly points to the |
This fixes a long standing issue where re-exported namespaces are not
disambiguated. If a namespace is re-exported in multiple dependencies,
then according to ecma262 this is not an ambiguous import.
Example:
export let x = 1
/ \
export * as dep from ... export * as dep from ...
\ /
export * from left
export * from right
|
import { dep } from ...
haz(dep.x)
While doing so we also implement the normative PR3715 [0] treating
`export * as dep from ...` the same as
`import * as dep from ...; export { dep }`.
Since modules can import themselves we must be able to produce a
shared binding cell to identify the namespace, before the namespace is
fully populated. To that end we store the namespace indirectly through
an additional binding cell in the module that we can use as a result
of the name resolution.
In other words in a module graph like
export let x = 1
|
export * as dep from ...
|
import { dep } from ...
We will now resolve `{ dep }` to the special namespace binding cell on
the topmost module instead of the private re-export binding cell of the
middle module.
[0] tc39/ecma262#3715
Bug: 464065358
Change-Id: Ief31089987085772f92f3c32aecbf02db6796cee
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7362765
Auto-Submit: Olivier Flückiger <[email protected]>
Commit-Queue: Leszek Swirski <[email protected]>
Reviewed-by: Leszek Swirski <[email protected]>
Cr-Commit-Position: refs/heads/main@{#104825}
Fixes #3710. Specifically, this makes it so that the example under "this throws" in this comment no longer throws, matching the behavior for re-exporting bindings rather than full namespaces.
I also added an explanatory comment about why we're doing this rewriting.
cc @nicolo-ribaudo @guybedford