Skip to content

Conversation

@asgerf
Copy link
Contributor

@asgerf asgerf commented Mar 3, 2023

Adds support for import assertions. For example:

import * as foo from 'foo' assert { type: 'json' }

For TypeScript these were tolerated but not extracted, and for plain JS they caused a parse error. Now they are supported and extracted in both dialects.

The language feature was recently downgraded to stage 2, but based on the discussion it seems unlikely that the syntax will change.

Evaluation was uneventful.

Commit-by-commit review recommended.

@asgerf asgerf force-pushed the js/import-assertion branch from a83329d to 7f96fe7 Compare March 3, 2023 11:21
@asgerf asgerf added the Awaiting evaluation Do not merge yet, this PR is waiting for an evaluation to finish label Mar 6, 2023
@asgerf asgerf marked this pull request as ready for review March 6, 2023 09:48
@asgerf asgerf requested a review from a team as a code owner March 6, 2023 09:48
@asgerf asgerf requested review from aibaars and removed request for a team March 6, 2023 09:48
@asgerf asgerf removed the Awaiting evaluation Do not merge yet, this PR is waiting for an evaluation to finish label Mar 7, 2023
@erik-krogh erik-krogh self-requested a review March 13, 2023 11:58
erik-krogh
erik-krogh previously approved these changes Mar 13, 2023
Copy link
Contributor

@erik-krogh erik-krogh left a comment

Choose a reason for hiding this comment

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

Really nice work 👍
And very easy to follow in a commit-by-commit review 👍

A single very optional comment.

Expression attributes = null;
if (this.eat(TokenType.comma)) {
attributes = this.parseMaybeAssign(false, null, null);
if (this.type != TokenType.parenR) { // Skip if the comma was a trailing comma
Copy link
Contributor

@erik-krogh erik-krogh Mar 13, 2023

Choose a reason for hiding this comment

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

Optional: Trailing commas in dynamic imports is not part of the tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a trap test

@asgerf asgerf requested a review from a team as a code owner March 14, 2023 12:15
@asgerf asgerf merged commit feb7c49 into github:main Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants