Normative: Add Import Attributes
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)
@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 :)
I pushed two more normative commits, that are oversights from the assert->with migration:
- Added
assertas an alternative towithalso inexport ... fromstatements - Removed the
[no LineTerminator here]restriction beforewith
I pushed two new commits:
Validate attrs when lodaing deps and not when parsingfixes https://github.com/tc39/proposal-import-attributes/issues/144. From an ECMA-262 point of view this is purely editorial, because:- it's not currently observable whether an error is thrown when parsing or when calling
LoadImportedModules() - code that resulted in a syntax error still results in a syntax error, and vice versa. there might be a difference in the location of the reported syntax error (see the example in the linked issue), but that's already completely implementation-defined.
- it's not currently observable whether an error is thrown when parsing or when calling
Merge AssertClause into WithClause, and fix missing SDOswhile fixing https://github.com/tc39/ecma262/pull/3057#pullrequestreview-1495910003 I realized that I had to duplicate every SDO usingWithClauseinto exactly the same definition but usingAssertClause. There are many of them, and copy-pasting risks introducing problems when we update one and forget to update the other one. I ended up merging the two productions as originally suggested by @bakkot in https://github.com/tc39/ecma262/pull/3057#discussion_r1225713248, but with a production parameter to only allowassertin the[no LineTerminator here]case.
Why not just tweak @bakkot's suggestion into:
AttributesKeyword :
`with`
[no LineTerminator here] `assert`
That would eliminate the [DeprecatedAssert] parameter and the deprecated definitions of ImportDeclaration and ExportDeclaration.
[no LineTerminator here] is never used at the beginning/end of a production currently, but if the editors like that I think it's a very good idea.
I pushed @jmdyck's suggestion, to show how much it would simplify the spec.
[no LineTerminator here]is never used at the beginning/end of a production currently,
A [no LineTerminator here] constraint is similar in flavor to a [lookahead ...] constraint, and we have lots of occurrences of those at the start + end of productions (4 at the start, 24 at the end).
And there's nothing in the definition of [no LineTerminator here] that would preclude its use at the start/end of a production.
So I don't think we should have any qualms about using it in this way.
I think the ModuleRequests SDO needs rule(s) to handle:
`import` ModuleSpecifier WithClause? `;`
PR updated to remove support for float and bigint literal keys, as per consensus in the TC39 2023-09 meeting.
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 https://github.com/tc39/ecma262/pull/3094 on top of this PR. What approach do you prefer?
- I push this PR's branch to a temporary branch in this repository, and we open PRs targeting it
- those PRs include commits from this one
- we open those PRs in my fork, and you review there
@nicolo-ribaudo I prefer 1 but 3 is also fine.
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.
I did it for you at https://github.com/tc39/ecma262/tree/import-attributes.
@jmdyck Done, thanks!
@tc39/ecma262-editors Remember that this is going for stage 4 at the next meeting :)
Could somebody restart CI?
1 commits have an unknown author: 57f427b18bf7e629565ac2fcf2392ba7b7d0d8fb
@guybedford Yeah the IPR check periodically gets out whack; don't worry about it. We'll fix before landing.
What's the status of this?
@linusg It's stage 4, editors just haven't had time to get it landed yet.
@bakkot thanks for the clarification - please just let us know if there's anything needed to help move this one along.
It's now been four months since this reached stage 4 and there doesn't seem to be any movement still. Could we get an update from the editors when they plan to address this PR please?
- It's blocking #3391 which is also stage 4
- I'm not sure about the exact cutoff date but it would be unfortunate if this doesn't make it into the ES 2025 snapshot despite being part of the language now
- I can't speak for any other small-scale engine implementors but personally I do want to implement both of these features but not before they have been integrated to avoid potential editorial churn
I'm not bringing this up formally at the upcoming plenary but I feel like there is a discussion to be had about requiring editorial sign-off before requesting stage 4 so that PRs can be merged without further thought afterwards.
Note that this PR has LGTMs from Michael (https://github.com/tc39/ecma262/pull/3057#pullrequestreview-1480381585), Kevin (https://github.com/tc39/ecma262/pull/3057#pullrequestreview-1473591443) and jmdyck (https://github.com/tc39/ecma262/pull/3057#pullrequestreview-2573923746).
After Michael's and Kevin's LGTMs, there has been the change to delete the part about the "legacy assert keyword", so the PR needs another quick review, but it should be almost ready to land at this point.
@linusg the cutoff is typically based on when stage 4 was achieved, and not based on when the PR is mergeable.
That's not what I mean - there will be a snapshot of the entire spec somewhere in the Ecma archives that says "this is ECMAScript 2025". If this PR is not merged by the time that snapshot is finalized import attributes and JSON modules won't be in there, despite having achieved stage 4. I was unable to find out when that happens exactly but I'm told it's around this time of the year.
@linusg i'm the one who makes that snapshot, and it's never made until all outstanding stage 4 PRs are merged, so you don't have to worry about it.
(it's also a living standard, so nobody should be caring about the snapshots, only what's merged into github :-) )
Absolutely, I don't use these snapshots ever. Still seemed worth pointing out to avoid this situation, thanks for clarifying!
I rebased on main and squashed and now esmeta is complaining, I'm checking why.
[ParamTypeMismatch] Call[16128] argument assignment to second parameter _request_ when Call[16128] function call from Record[SourceTextModuleRecord].GetExportedNames (step 8.b, 19:43-93) to GetImportedModule
- expected: Record[ModuleRequestRecord]
- actual : String
[ParamTypeMismatch] Call[16163] argument assignment to second parameter _request_ when Call[16163] function call from Record[SourceTextModuleRecord].ResolveExport (step 6.a.ii, 16:44-94) to GetImportedModule
- expected: Record[ModuleRequestRecord]
- actual : String
[ParamTypeMismatch] Call[16178] argument assignment to second parameter _request_ when Call[16178] function call from Record[SourceTextModuleRecord].ResolveExport (step 9.b, 31:42-92) to GetImportedModule
- expected: Record[ModuleRequestRecord]
- actual : String
[ParamTypeMismatch] Call[16215] argument assignment to second parameter _request_ when Call[16215] function call from Record[SourceTextModuleRecord].InitializeEnvironment (step 7.a, 13:42-93) to GetImportedModule
- expected: Record[ModuleRequestRecord]
- actual : String
These are wrong. It thinks ExportEntry's [[ModuleRequest]] is a String, but this PR updates it to be a ModuleRequest Record.
[ParamTypeMismatch] Call[16296] argument assignment to first parameter _left_ when Call[16296] function call from FinishLoadingImportedModule (step 1.a, 3:105-151) to ModuleRequestsEqual
- expected: Record[LoadedModuleRequestRecord | ModuleRequestRecord]
- actual : Record[{ Module : Record[ModuleRecord | ModuleRecordN], Specifier : String }]
This is also wrong. The actual type is LoadedModuleRequestRecord, declared in the line before.
[ParamTypeMismatch] Call[9455] argument assignment to first parameter _O_ when Call[9455] function call from EvaluateImportCall (step 11.d.iv.1, 28:35-54) to Get
- expected: Record[Object]
- actual : ESValue
[ParamTypeMismatch] Call[9457] argument assignment to first parameter _O_ when Call[9457] function call from EvaluateImportCall (step 11.d.iv.2, 29:37-56) to Get
- expected: Record[Object]
- actual : ESValue
These are because esmeta doesn't know that the entries of the list returned by EnumerableOwnProperties are arrays (thus objects).
[ReturnTypeMismatch] Block[9363] return statement in EvaluateImportCall (step 3, 4:36-73)
- expected: Normal[Record[Promise]] | Throw
- actual : Return | Throw
[ReturnTypeMismatch] Block[9376] return statement in EvaluateImportCall (step 5.a, 7:36-71)
- expected: Normal[Record[Promise]] | Throw
- actual : Return | Throw
This caught a bug! In case of import(yield), evaluating yield can return, thus EvaluateImportCall's type definition needs to allow Return completions.
I'll edit the PR fixing this last bug, and ignoring the other errors.