Skip to content

feat: add phase field to ImportExpression in TS-ESTree AST#36

Merged
overlookmotel merged 2 commits intomainfrom
05-20-feat_add_phase_field_to_importexpression_in_ts-estree_ast
May 20, 2025
Merged

feat: add phase field to ImportExpression in TS-ESTree AST#36
overlookmotel merged 2 commits intomainfrom
05-20-feat_add_phase_field_to_importexpression_in_ts-estree_ast

Conversation

@overlookmotel
Copy link
Member

@overlookmotel overlookmotel commented May 20, 2025

Add a field phase to ImportExpression in TS-ESTree AST, to support oxc-project/oxc#10978.

This field is always null because none of the test cases use import.defer(...) or import.source(...).

Copy link
Member Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@overlookmotel overlookmotel marked this pull request as ready for review May 20, 2025 18:01
@overlookmotel overlookmotel merged commit bf1f5de into main May 20, 2025
2 checks passed
@overlookmotel overlookmotel deleted the 05-20-feat_add_phase_field_to_importexpression_in_ts-estree_ast branch May 20, 2025 20:02
graphite-app bot pushed a commit to oxc-project/oxc that referenced this pull request May 21, 2025
…e AST (#11193)

Related to #10978. Add `phase` property to `ImportExpression` in TS-ESTree AST.

This aligns with ESTree spec, but does *not* align with TS-ESLint - TS-ESLint outputs a `CallExpression` with a `MetaProperty` as its `callee` instead.

We don't have the necessary information in our AST to match TS-ESLint, because we don't have the spans of `import`, `defer`, or `import.defer`, to use for the `MetaProperty`, or the `IdentifierName`s it contains.

This is not completely ideal, but I figure it's preferable to represent this syntax *somehow* in the AST, rather than completely ignoring it, in which case there's no way to distinguish between `import("x")`, `import.defer("x")`, and `import.source("x")`.

Presumably TS-ESLint will change their AST shape to conform to ESTree spec once these reach stage 4, so I don't think we should expend effort changing our AST to align with them in the meantime. TypeScript doesn't even have test cases for these (we'd know if they did, as we'd be failing them) - so I doubt these are popular constructs in TS.

`acorn-test262` submodule is bumped to include oxc-project/estree-conformance#36 which adds the `phase` field to `ImportExpression`s in test snapshots.
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.

1 participant