Skip to content
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

BUG: Module namespace objects have undefined Indirect exports #5501

Closed
rhuanjl opened this issue Jul 23, 2018 · 1 comment · Fixed by #5780
Closed

BUG: Module namespace objects have undefined Indirect exports #5501

rhuanjl opened this issue Jul 23, 2018 · 1 comment · Fixed by #5780
Assignees
Milestone

Comments

@rhuanjl
Copy link
Collaborator

rhuanjl commented Jul 23, 2018

There is a bug in the initialisation of module namespace objects when the module contains an indirect export.

Simple proof of concept

//test.mjs
export {bar as foo} from "./test.mjs";
export const bar = 5;
import * as stuff from "./test.mjs";

print(JSON.stringify(stuff)); //prints {"bar":5} - should print {"bar":5, "foo":5}
print(Object.getOwnPropertyNames(stuff)); // correctly prints "bar, foo"
print(stuff.foo); // prints undefined - should print 5

Credit @fatcerberus for finding that there was a CC bug here

EDIT: deleted info on where I thought the bug came from as I think what I'd posted was wrong.

@rhuanjl
Copy link
Collaborator Author

rhuanjl commented Oct 14, 2018

Found a solution for this whilst working on 5779.

chakrabot pushed a commit that referenced this issue Oct 31, 2018
Merge pull request #5779 from rhuanjl:exportNamespace

This PR does the following:
1. implements support for `export * as ns from "module"` syntax a normative change PR to ecma262 which has tc39 consensus
See spec diff: https://spectranaut.github.io/proposal-export-ns-from/
See normative PR here: tc39/ecma262#1174 (comment)
This is placed behind a flag but set to default enabled
2. Adds some relevant tests
3. Fixes a bug where duplicate exports weren't always a syntax error (implementing this feature correctly without fixing this bug would have been awkward)
4. Some drive by syntax error message improvement for duplicate exports and alias'd exports
    - "Syntax error: Syntax error" -> "Syntax error: Duplicate export of name '%s'"
    - "Syntax error: Syntax error" -> "Syntax error: 'as' is only valid if followed by an identifier."

There are unfortunately some remaining related test262 failures due to #5778 and #5501

closes #5759
fixes #5777
chakrabot pushed a commit that referenced this issue Oct 31, 2018
Merge pull request #5780 from rhuanjl:indirectExports

Saw the solution to this whilst working on #5779

@digitalinfinity conscious the issue for this (5501) was assigned to you, was intending to leave but saw the fix whilst looking at the code for something else. Let me know if this PR is unwanted and I will close it.

This fixes two cases where a module-namespace object could have undefined indirect exports:
1. `export {a as b} from "thisModule";` - due to it not being in the localExports list but then being marked as a local export when processing the indirectExports
2. `export {default as otherName} from "anyModule";` - due to seemingly unnecessary special handling of `default`

Test262 note, this fix introduces passes for 6 es6 module tests, it is also necessary for several Dynamic Import tests to pass - and for some `export * as ns` tests. The 6 es6 tests that this fixes are:
test/language/module-code/namespace/internals/get-own-property-str-found-init.js
test/language/module-code/namespace/internals/get-str-initialize.js
test/language/module-code/namespace/internals/get-str-found-init.js
test/language/module-code/namespace/internals/get-str-update.js
test/language/module-code/namespace/internals/has-property-str-found-uninit.js
test/language/module-code/namespace/internals/has-property-str-found-init.js

fix #5501
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants