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

Implement export-ns-from syntax. #5759

Closed
jdalton opened this issue Oct 5, 2018 · 7 comments · Fixed by #5779
Closed

Implement export-ns-from syntax. #5759

jdalton opened this issue Oct 5, 2018 · 7 comments · Fixed by #5779

Comments

@jdalton
Copy link

jdalton commented Oct 5, 2018

Now that export-ns-from has TC39 consensus the TC39 is looking for engines to implement.

Related: V8 issue and SpiderMonkey issue

@rhuanjl
Copy link
Collaborator

rhuanjl commented Oct 9, 2018

Would a PR to implement this be appreciated?

@jdalton
Copy link
Author

jdalton commented Oct 10, 2018

@rhuanjl Yes, totally!

@rhuanjl
Copy link
Collaborator

rhuanjl commented Oct 10, 2018

@jdalton I was looking for an answer from the CC team as conscious that if i write an implementation for this (as an external contributor) one of them has to review it, test it and merge it

@jackhorton
Copy link
Contributor

cc @boingoing

I haven't heard anyone say they were picking this up yet internally so it should be good to go.

@boingoing
Copy link
Contributor

@rhuanjl We would definitely appreciate a contribution from you on this one and I would be happy to review it and marshal it through the checkin process.

Just to clarify what exactly this feature is, it's re-exporting a namespace object under an exported name?

@leobalter
Copy link

@boingoing: Just to clarify what exactly this feature is, it's re-exporting a namespace object under an exported name?

yes! The syntax production was not possible before:

export * as ns from './module.js';

which is almost a sugar for:

import * as ns from './module.js';
export { ns };

without necessarily importing a local ns binding.

I hope the tests from Test262 might help. @rhuanjl please let me know if you need any help from the tests.

@rhuanjl
Copy link
Collaborator

rhuanjl commented Oct 11, 2018

Thanks all, I'll aim to do this over this weekend.

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
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.

6 participants