Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(ast): remove JsImportNamespaceClause #431

Closed

Conversation

Conaclos
Copy link
Member

Summary

While I was working on #428, I noticed that a namespace import, such as import * as X from "", can be represented in two ways. One way uses JsImportNamesapceClause and the other use JsNamespaceImportSpecifier embedded inside a JsNamedImportClause with a missing JsDefaultImportSpecifier.

This PR removes JsImportNamespaceClause and ensure that we have a unique way of representing a namespace import.

I think we should refactor the import AST nodes further. There is room for better factorization.

Test Plan

Updated snapshots.

@Conaclos Conaclos temporarily deployed to Website deployment September 27, 2023 10:35 — with GitHub Actions Inactive
@github-actions github-actions bot added A-Linter Area: linter A-Parser Area: parser A-Formatter Area: formatter A-Tooling Area: internal tools L-JavaScript Language: JavaScript and super languages labels Sep 27, 2023
@github-actions
Copy link
Contributor

Parser conformance results on

js/262

Test result main count This PR count Difference
Total 48863 48863 0
Passed 47808 47808 0
Failed 1055 1055 0
Panics 0 0 0
Coverage 97.84% 97.84% 0.00%

jsx/babel

Test result main count This PR count Difference
Total 40 40 0
Passed 37 37 0
Failed 3 3 0
Panics 0 0 0
Coverage 92.50% 92.50% 0.00%

symbols/microsoft

Test result main count This PR count Difference
Total 6212 6212 0
Passed 1767 1767 0
Failed 4445 4445 0
Panics 0 0 0
Coverage 28.44% 28.44% 0.00%

ts/babel

Test result main count This PR count Difference
Total 639 639 0
Passed 571 571 0
Failed 68 68 0
Panics 0 0 0
Coverage 89.36% 89.36% 0.00%

ts/microsoft

Test result main count This PR count Difference
Total 17224 17224 0
Passed 13123 13123 0
Failed 4101 4101 0
Panics 0 0 0
Coverage 76.19% 76.19% 0.00%

@Conaclos Conaclos marked this pull request as draft September 27, 2023 15:07
@Conaclos
Copy link
Member Author

Conaclos commented Oct 1, 2023

I am closing this PR because I think my #436 is better

@Conaclos Conaclos closed this Oct 1, 2023
@Conaclos Conaclos deleted the conaclos/ast/remove-js-import-namespace-clause branch October 18, 2023 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Formatter Area: formatter A-Linter Area: linter A-Parser Area: parser A-Tooling Area: internal tools L-JavaScript Language: JavaScript and super languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant