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

Normative: Add Import Attributes #3057

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Normative: Add Import Attributes #3057

wants to merge 2 commits into from

Conversation

nicolo-ribaudo
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo commented May 8, 2023 •
edited
Loading

This proposal (https://tc39.es/proposal-import-attributes/, https://github.com/tc39/proposal-import-attributes) reached stage 3 during the March 2023 meeting, conditional on stage 3 reviews. I'm opening this PR before "real" stage 3 to help editors reviewing the full spec changes.

The proposal has already been integrated in HTML: https://html.spec.whatwg.org/#hostloadimportedmodule

PREVIEW: https://ci.tc39.es/preview/tc39/ecma262/pull/3057
DIFF: https://arai-a.github.io/ecma262-compare/?pr=3057 (note: the diff doesn't mark deprecated sections)

nicolo-ribaudo
nicolo-ribaudo commented May 8, 2023
View reviewed changes
spec.html Outdated Show resolved Hide resolved
nicolo-ribaudo
nicolo-ribaudo commented May 8, 2023
View reviewed changes
spec.html Outdated Show resolved Hide resolved
nicolo-ribaudo
nicolo-ribaudo commented May 8, 2023
View reviewed changes
spec.html Outdated Show resolved Hide resolved
@nicolo-ribaudo nicolo-ribaudo mentioned this pull request May 8, 2023
Closed
3 tasks
@nicolo-ribaudo nicolo-ribaudo added normative change Affects behavior required to correctly evaluate some ECMAScript source text pending stage 4 This proposal has not yet achieved stage 4, but may otherwise be ready to merge. proposal This is related to a specific proposal, and will be closed/merged when the proposal reaches stage 4. needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 and removed pending stage 4 This proposal has not yet achieved stage 4, but may otherwise be ready to merge. labels May 8, 2023
jmdyck
jmdyck requested changes May 8, 2023
View reviewed changes
spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
spec.html Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
@nicolo-ribaudo nicolo-ribaudo mentioned this pull request May 10, 2023
Closed
jhnaldo
jhnaldo reviewed May 18, 2023
View reviewed changes
spec.html Outdated Show resolved Hide resolved
@nicolo-ribaudo nicolo-ribaudo mentioned this pull request May 18, 2023
Closed
jmdyck
jmdyck reviewed May 18, 2023
View reviewed changes
spec.html Outdated Show resolved Hide resolved
@nicolo-ribaudo
Copy link
Member Author

nicolo-ribaudo commented May 22, 2023 •
edited
Loading

@tc39/ecma262-editors Currently the only remaining condition for this proposal to be actually Stage 3 is the editorial review. It'd be great if you have time to do it :)

@bakkot bakkot added the editor call to be discussed in the next editor call label May 24, 2023
@ljharb ljharb force-pushed the import-attributes branch from 891ceab to 2521cf5 Compare May 31, 2023 21:41
@lucacasonato lucacasonato mentioned this pull request Jun 8, 2023
Open
bakkot
bakkot reviewed Jun 11, 2023
View reviewed changes
Copy link
Contributor

@bakkot bakkot left a comment

Choose a reason for hiding this comment

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

LGTM other than comments.

spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
spec.html Show resolved Hide resolved
@nicolo-ribaudo
Copy link
Member Author

I pushed two more normative commits, that are oversights from the assert->with migration:

  1. Added assert as an alternative to with also in export ... from statements
  2. Removed the [no LineTerminator here] restriction before with

michaelficarra
michaelficarra reviewed Jun 13, 2023
View reviewed changes
spec.html Outdated Show resolved Hide resolved
michaelficarra
michaelficarra reviewed Jun 13, 2023
View reviewed changes
spec.html Outdated Show resolved Hide resolved
michaelficarra
michaelficarra reviewed Jun 13, 2023
View reviewed changes
spec.html Outdated Show resolved Hide resolved
michaelficarra
michaelficarra reviewed Jun 13, 2023
View reviewed changes
spec.html Outdated Show resolved Hide resolved
michaelficarra
michaelficarra reviewed Jun 13, 2023
View reviewed changes
spec.html Outdated Show resolved Hide resolved
michaelficarra
michaelficarra reviewed Jun 13, 2023
View reviewed changes
spec.html Outdated Show resolved Hide resolved
michaelficarra
michaelficarra reviewed Jun 13, 2023
View reviewed changes
spec.html Outdated Show resolved Hide resolved
michaelficarra
michaelficarra reviewed Jun 13, 2023
View reviewed changes
spec.html Outdated Show resolved Hide resolved
michaelficarra
michaelficarra reviewed Jun 13, 2023
View reviewed changes
spec.html Outdated Show resolved Hide resolved
@nicolo-ribaudo nicolo-ribaudo removed the needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 label Jun 9, 2024
@arnemolland arnemolland mentioned this pull request Jun 19, 2024
Closed
@chybisov chybisov mentioned this pull request Jul 30, 2024
Merged
@nicolo-ribaudo
Copy link
Member Author

PR updated to remove the assert variant, as per consensus in the TC39 2024-07 meeting.

@tc39/ecma262-editors I'd like to open a PR for https://tc39.es/proposal-json-modules/, which I plan to propose for stage 4 at the next meeting. The PR for that proposal needs to be based on top of this one. Guy would also like to rebase #3094 on top of this PR. What approach do you prefer?

  1. I push this PR's branch to a temporary branch in this repository, and we open PRs targeting it
  2. those PRs include commits from this one
  3. we open those PRs in my fork, and you review there

@michaelficarra
Copy link
Member

@nicolo-ribaudo I prefer 1 but 3 is also fine.

@nicolo-ribaudo
Copy link
Member Author

It turns out I don't have push access to this repo. Could somebody (@michaelficarra? @ljharb?) push this PR's branch to this repo so I can open a PR targeting it? :) Or give me push access, I don't know what's the policy.

@ryzokuken
Copy link
Member

I did it for you at https://github.com/tc39/ecma262/tree/import-attributes.

@nicolo-ribaudo nicolo-ribaudo mentioned this pull request Aug 14, 2024
Open
jmdyck
jmdyck requested changes Aug 16, 2024
View reviewed changes
spec.html Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
spec.html Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
spec.html Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
spec.html Show resolved Hide resolved
@Westbrook Westbrook mentioned this pull request Aug 17, 2024
Closed
@nicolo-ribaudo
Copy link
Member Author

@jmdyck Done, thanks!

@nicolo-ribaudo
Copy link
Member Author

nicolo-ribaudo commented Sep 24, 2024 •
edited
Loading

@tc39/ecma262-editors Remember that this is going for stage 4 at the next meeting :)

@nicolo-ribaudo nicolo-ribaudo added has stage 4 This PR represents a proposal that has achieved stage 4, and is ready to merge. and removed pending stage 4 This proposal has not yet achieved stage 4, but may otherwise be ready to merge. labels Oct 9, 2024
@nicolo-ribaudo

This comment was marked as resolved.

Sign in to view
nicolo-ribaudo and others added 2 commits October 10, 2024 06:35
@nicolo-ribaudo @michaelficarra
Add Import Attributes proposal
2786b20 
* Add Import Attributes proposal
* `npm run format`
* Updates from review
* Changes from review
* Do not re-define ImportDeclaration
* Add `assert` deprecated syntax to `export ... from`
* Remove `[no LineTerminator here]` before `with`
* Use optional symbols to reduce grammar
* Update ImportEntries and ExportEntry to use ModuleRequest Records
* Review from Michael
* Replace AttributeKey with LiteralPropertyName
* Separate ModuleRequest and LoadedModuleRequest fields
* Validate attrs when lodaing deps and not when parsing
* Merge `AssertClause` into `WithClause`, and fix missing SDOs
* Fix type annotation
* Simplify AttributesKeyword
* Reviews
* Review
* Remove support for float and bigint literal keys
* Update wording
@nicolo-ribaudo @michaelficarra
Review by jmdyck
93f3d26
@mdjermanovic mdjermanovic mentioned this pull request Oct 13, 2024
Closed
10 tasks
@guybedford

This comment was marked as resolved.

Sign in to view
@bakkot

This comment was marked as resolved.

Sign in to view
@linusg
Copy link
Member

linusg commented Nov 20, 2024

What's the status of this?

@bakkot
Copy link
Contributor

bakkot commented Nov 20, 2024

@linusg It's stage 4, editors just haven't had time to get it landed yet.

@guybedford guybedford mentioned this pull request Nov 21, 2024
Open
@029A-h 029A-h mentioned this pull request Dec 5, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lucacasonato lucacasonato lucacasonato left review comments

@jhnaldo jhnaldo jhnaldo left review comments

@bakkot bakkot bakkot left review comments

@michaelficarra michaelficarra michaelficarra approved these changes

@jmdyck jmdyck jmdyck requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
has stage 4 This PR represents a proposal that has achieved stage 4, and is ready to merge. has test262 tests normative change Affects behavior required to correctly evaluate some ECMAScript source text proposal This is related to a specific proposal, and will be closed/merged when the proposal reaches stage 4.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@nicolo-ribaudo @jmdyck @michaelficarra @ryzokuken @guybedford @bakkot @linusg @jhnaldo @lucacasonato