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.

Sign up for GitHub

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

Jump to bottom

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 •
edited by mathiasbynens
Loading

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
bterlson
bterlson approved these changes Sep 19, 2017
View reviewed changes
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 added a commit to tc39/proposals that referenced this pull request Sep 26, 2017
@ljharb
Remove export-ns-from since it moved to tc39/ecma262#1005
77d957b
@Jamesernator Jamesernator mentioned this pull request Nov 12, 2017
Open
@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.

@lukescott lukescott mentioned this pull request Mar 1, 2018
Closed
@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 spectranaut mentioned this pull request Mar 19, 2018
Merged
@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 spectranaut mentioned this pull request Apr 17, 2018
Merged
@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.

@jdalton jdalton closed this Apr 17, 2018
This was referenced Jul 17, 2018
Open
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bterlson bterlson bterlson approved these changes

Assignees

@leobalter leobalter

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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jdalton @ljharb @spectranaut @leobalter @bterlson