Parse import reflection by JLHwung · Pull Request #14926 · babel/babel

JLHwung · 2022-09-13T20:10:16Z

Q	A
Fixed Issues?	Implement parsing / printing support of the stage-2 Import reflection proposal
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?	Yes
Tests Added + Pass?	Yes
Documentation PR Link	babel/website#2684
Any Dependency Changes?
License	MIT

This PR also introduces the new syntax plugin and adds it to babel-standalone. The AST shape is still preliminary, for discussions about the AST shape, please visit estree/estree#287.

I will update the PR should the AST change.

babel-bot · 2022-09-13T20:18:55Z

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/53242/

nicolo-ribaudo

Does this PR preserve parsing of import module from "x"?

JLHwung · 2022-09-20T19:03:38Z

Does this PR preserve parsing of import module from "x"?

Good catch. I thought tt._module were a keyword before. I am working on a fix.

nicolo-ribaudo · 2022-09-21T07:42:47Z

(fyi @lucacasonato @guybedford)

nicolo-ribaudo

How does this new plugin interact with import assertions?

Can we throw if both module and asserts as used in the same declaration, while we wait for the two proposals to decide how to integrate?

...l-parser/test/fixtures/experimental/import-reflection/invalid-flow-type-import-2/output.json

packages/babel-parser/src/parse-error/standard-errors.ts

JLHwung · 2022-09-21T14:03:24Z

packages/babel-types/src/ast-types/generated/index.ts

  source: StringLiteral;
  assertions?: Array<ImportAttribute> | null;
  importKind?: "type" | "typeof" | "value" | null;
+  module?: boolean | null;


t.ImportDeclaration will assign null to module because it is an optional property.

liuxingbaoyu · 2022-09-21T14:27:56Z

packages/babel-generator/src/generators/modules.ts

 export function ImportDeclaration(this: Printer, node: t.ImportDeclaration) {
  this.word("import");
  this.space();
+  this.printInnerComments(node);


Is it possible to use indent=false here?

nicolo-ribaudo · 2022-10-04T16:49:09Z

packages/babel-parser/src/parser/statement.ts

+          if (nextNextTokenFirstChar === charCodes.lowercaseF) {
+            // import module from from ...
+            isImportReflection = true;


Can you add a test for import module from foo, just to verify that it errors even if foo starts with f?

nicolo-ribaudo · 2022-10-04T16:49:46Z

packages/babel-parser/src/parser/statement.ts

+        // import module { x } ...
+        // This is invalid, we will continue parsing and throw
+        // a recoverable error later
+        isImportReflection = true;


Can you also add a test for import module "foo"?

nicolo-ribaudo · 2022-10-24T21:45:39Z

ping

Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>

liuxingbaoyu · 2022-10-25T18:32:10Z

It doesn't seem to be expected here.

JLHwung · 2022-10-25T19:00:44Z

It doesn't seem to be expected here.

It is expected but not ideal, because an import declaration has more than one whitespace that can not be attached to any adjacent AST nodes:

import /* 1 */ { x } /* 2 */ from "foo" assert /* 3 */ { type: "json" };
import /* 1 */ type x from "foo";

In this example, all comments are inner comments of the import declaration. But the generator does not know where it should print inner comments. Currently what we can do is to infer such position from the AST structure (whether the specifier is an ImportSpecifier, whether it contains import assertions, etc). To precisely regenerate sources from AST, we will need structures more refined than innerComments or change the AST to avoid such whitespaces.

liuxingbaoyu · 2022-10-25T19:09:01Z

I mean the comment positions are being swapped, which is kind of weird.

nicolo-ribaudo · 2022-10-26T16:43:15Z

...babel-parser/test/fixtures/experimental/import-reflection/invalid-module-source/options.json

@@ -0,0 +1,3 @@
+{
+  "throws": "Unexpected token, expected \"{\" (1:14)"


This error isn't the best, since { is not allowed here, but we can iterate on it.

JLHwung added PR: New Feature 🚀 A type of pull request used for our changelog categories Spec: Source Phase Imports labels Sep 13, 2022

JLHwung force-pushed the parse-import-reflection branch 2 times, most recently from 0fdd80c to 4f9b91e Compare September 13, 2022 20:15

JLHwung force-pushed the parse-import-reflection branch from 4f9b91e to f6ba14e Compare September 20, 2022 15:27

nicolo-ribaudo reviewed Sep 20, 2022

View reviewed changes

JLHwung force-pushed the parse-import-reflection branch 2 times, most recently from 612d89c to a81f8bc Compare September 20, 2022 19:55

nicolo-ribaudo reviewed Sep 21, 2022

View reviewed changes

...l-parser/test/fixtures/experimental/import-reflection/invalid-flow-type-import-2/output.json Outdated Show resolved Hide resolved

packages/babel-parser/src/parse-error/standard-errors.ts Outdated Show resolved Hide resolved

liuxingbaoyu added this to the 7.20.0 milestone Sep 21, 2022

liuxingbaoyu reviewed Sep 21, 2022

View reviewed changes

JLHwung force-pushed the parse-import-reflection branch from acc7cb4 to 3f9f6c8 Compare September 21, 2022 14:01

liuxingbaoyu reviewed Sep 21, 2022

View reviewed changes

liuxingbaoyu approved these changes Sep 21, 2022

View reviewed changes

nicolo-ribaudo reviewed Oct 4, 2022

View reviewed changes

nicolo-ribaudo added the PR: Needs Docs label Oct 4, 2022

JLHwung and others added 10 commits October 24, 2022 18:02

chore: improve token typings

3a45f53

parse import reflection

7031a3c

print import reflection

e5780ec

add syntax plugin

4a53088

add syntax plugin to standalone

9f48d7d

update AST to module: boolean

bacf8d2

fix: module is not a reserved JS keyword

178538d

fix: print ImportDeclarations' inner comments

2cc6536

cleanup

05fe1e2

Update packages/babel-parser/src/parse-error/standard-errors.ts

d63f44b

Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>

JLHwung added 4 commits October 24, 2022 18:09

Disable import assertions after import module

f363a56

Update flow/ts integration errors

98c8a28

avoid indent when printing import's inner comments

470f301

add more test cases

9467184

JLHwung force-pushed the parse-import-reflection branch from bdf52e8 to 9467184 Compare October 24, 2022 22:09

update generated files

9f14643

fix: print inner comments before type/module

47683a3

liuxingbaoyu approved these changes Oct 26, 2022

View reviewed changes

nicolo-ribaudo approved these changes Oct 26, 2022

View reviewed changes

nicolo-ribaudo added PR: Ready to be Merged A pull request with already two approvals, but waiting for the next minor release and removed PR: Needs Docs labels Oct 26, 2022

nicolo-ribaudo merged commit dfc4b61 into babel:main Oct 26, 2022

nicolo-ribaudo deleted the parse-import-reflection branch October 26, 2022 19:35

JLHwung mentioned this pull request Nov 16, 2022

fix: parse import module, ... #15198

Merged

shaneomatz mentioned this pull request Nov 18, 2022

[Snyk] Upgrade @babel/runtime from 7.12.5 to 7.20.0 shaneomatz/video.js#1

Open

github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Jan 26, 2023

github-actions bot locked as resolved and limited conversation to collaborators Jan 26, 2023

		@@ -0,0 +1,3 @@
		{
		"throws": "Unexpected token, expected \"{\" (1:14)"

Uh oh!

Conversation

JLHwung commented Sep 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

babel-bot commented Sep 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nicolo-ribaudo left a comment

Choose a reason for hiding this comment

Uh oh!

JLHwung commented Sep 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nicolo-ribaudo commented Sep 21, 2022

Uh oh!

nicolo-ribaudo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

JLHwung Sep 21, 2022

Choose a reason for hiding this comment

Uh oh!

liuxingbaoyu Sep 21, 2022

Choose a reason for hiding this comment

Uh oh!

nicolo-ribaudo Oct 4, 2022

Choose a reason for hiding this comment

Uh oh!

nicolo-ribaudo Oct 4, 2022

Choose a reason for hiding this comment

Uh oh!

nicolo-ribaudo commented Oct 24, 2022

Uh oh!

liuxingbaoyu commented Oct 25, 2022

Uh oh!

JLHwung commented Oct 25, 2022

Uh oh!

liuxingbaoyu commented Oct 25, 2022

Uh oh!

nicolo-ribaudo Oct 26, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

JLHwung commented Sep 13, 2022 •

edited

Loading

babel-bot commented Sep 13, 2022 •

edited

Loading

JLHwung commented Sep 20, 2022 •

edited

Loading