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

Add export * as ns from "mod” updates. #1005

Closed
wants to merge 1 commit into from
Closed

Add export * as ns from "mod” updates. #1005

wants to merge 1 commit into from

Conversation

jdalton
Copy link
Member

@jdalton jdalton commented Sep 18, 2017

This PR adds export * as ns from "mod" updates
from tc39/proposal-export-ns-from to the spec as part of the July 27, 2017 notes.

Tests to tc39/test262 soon to follow. \cc @benjamn

@ljharb ljharb added needs consensus This needs committee consensus before it can be eligible to be merged. normative change Affects behavior required to correctly evaluate some ECMAScript source text needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 labels Sep 19, 2017
spec.html Outdated
<emu-alg>
1. Let exportName be the StringValue of IdentifierName.
1. Let _entry_ be the ExportEntry Record {[[ModuleRequest]]: _module_, [[ImportName]]: `"*"`, [[LocalName]]: *null*, [[ExportName]]: _exportName_ }.
1. Return a new List containing _entry_
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

period at the end

spec.html Outdated
<emu-grammar>ExportFromClause : `*`</emu-grammar>
<emu-alg>
1. Let _entry_ be the ExportEntry Record {[[ModuleRequest]]: _module_, [[ImportName]]: `"*"`, [[LocalName]]: *null*, [[ExportName]]: *null* }.
1. Return a new List containing _entry_
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

period at the end

@ljharb
Copy link
Member

ljharb commented Mar 1, 2018

@jdalton i believe this needs tests and a rebase, and then it can come back to the committee for consensus?

@jdalton
Copy link
Member Author

jdalton commented Mar 1, 2018

Yes. I think @leobalter is on task for tests.

@spectranaut
Copy link

Hi @jdalton can you review this PR? tc39/proposal-export-ns-from#9
I think there is a bug in the spec!

By the way I'm writing these test instead of @leobalter, expect next week :)

@jdalton
Copy link
Member Author

jdalton commented Mar 15, 2018

can you review this PR?

Will do!

By the way I'm writing these test instead of @leobalter, expect next week :)

Thank you!

@spectranaut
Copy link

Hey @jdalton -- when reviewing the spec changes I found them to be incomplete! More work needs to be done here, I tried to outline the problem clearly in this issue: tc39/proposal-export-ns-from#10

Probably this PR should be closed, and a new one opened after finishing the proposal?

@ljharb
Copy link
Member

ljharb commented Mar 21, 2018

Please leave this one open, and update it as needed.

@leobalter
Copy link
Member

We lost the reference of the source branch/fork of this PR, so we can’t update this same PR. I told Val the best workaround would be importing this PR content and add other commits in another PR. I can make some to keep the original author info if it helps.

@jdalton
Copy link
Member Author

jdalton commented Mar 22, 2018

@leobalter No worries, do what's easiest. I don't care about the author cred for the PR.

@spectranaut
Copy link

I'll draft some spec changes to the appropriate methods (outlined here: tc39/proposal-export-ns-from#10) and open a new PR next week!

@spectranaut
Copy link

Before I open the new PR, would anyone like to review the changes in the pretty diff form: https://spectranaut.github.io/proposal-export-ns-from/

@ljharb
Copy link
Member

ljharb commented Apr 17, 2018

@spectranaut looks great to me (altho it should get many more eyes, and I'm not great on grammar/syntax stuff), i have a few small comments/questions but they can wait for the PR.

@spectranaut
Copy link

@ljharb, @leobalter, @jdalton : I made a new PR: #1174

@jdalton
Copy link
Member Author

jdalton commented Apr 17, 2018

Moved to #1174.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs consensus This needs committee consensus before it can be eligible to be merged. needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 normative change Affects behavior required to correctly evaluate some ECMAScript source text
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants