Skip to content

Add test for newline before WithClause - #5147

Open
tuankiet65 wants to merge 1 commit into
tc39:mainfrom
tuankiet65:import-attribute-after-newline
Open

tuankiet65 wants to merge 1 commit into
tc39:mainfrom
tuankiet65:import-attribute-after-newline

Conversation

@tuankiet65

Copy link
Copy Markdown

This commit adds test for newline before WithClause in ImportDeclaration and ExportDeclaration. This was allowed in tc39/proposal-import-attributes#136.

This commit adds test for newline before WithClause in ImportDeclaration
and ExportDeclaration. This was allowed since [1].

[1]: tc39/proposal-import-attributes#136

@hexbinoct hexbinoct left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I ran this locally. On node 24.14.0 (V8) the test passes, and engine262 parses it as a module. To check the newline is what the test depends on, I put a ; after each module specifier. Both engines then reject the file, because the next line becomes a with statement, which is not allowed in module code. Changing the expected 262.1 makes the assertion fail on V8.

The spec matches: ImportDeclaration and ExportDeclaration have WithClause? right after the specifier, with no [no LineTerminator here].

allow-nlt-before-with.js in the same directory already covers a newline before with, but only for import * as x from and only as a parse check. This one adds the bare import, a default import that is actually evaluated, and both export ... from forms, so I think they complement each other.

Two small things:

  • globalThis in features isn't used by this test.
  • The info block could show WithClause?, as the spec does. On that note, import-attribute-newlines.js still says import ModuleSpecifier[no LineTerminator here] WithClause; in its info, which is the old restriction this test shows is gone. That could be fixed here too if you like.

Looks good to me otherwise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants