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

feat(ast): implement new proposal-import-attributes #1476

Merged
merged 4 commits into from
Nov 25, 2023

Conversation

magic-akari
Copy link
Contributor

@magic-akari magic-akari commented Nov 21, 2023

@magic-akari
Copy link
Contributor Author

Current dependencies on/for this PR:

This stack of pull requests is managed by Graphite.

@github-actions github-actions bot added A-parser Area - Parser A-ast Area - AST A-codegen Area - Code Generation labels Nov 21, 2023
@Boshen
Copy link
Member

Boshen commented Nov 21, 2023

Is this another stage3 feature without test262 tests? We'll leave this as draft if this is the case :-(

@Boshen
Copy link
Member

Boshen commented Nov 21, 2023

I think I should write a rule for what ecma features we support at some point :-/

@magic-akari
Copy link
Contributor Author

magic-akari commented Nov 21, 2023

I implemented it after reading the TypeScript Release Note 🤣

@Boshen
Copy link
Member

Boshen commented Nov 21, 2023

I implemented it after reading the TypeScript Release Note 🤣

Stage 3 features can only be advanced to stage 4 when babel / typescript have implementations. I'm being more conservative around stage 3 features.

@magic-akari
Copy link
Contributor Author

Copy link

codspeed-hq bot commented Nov 24, 2023

CodSpeed Performance Report

Merging #1476 will not alter performance

Comparing ast/proposal-import-attributes (7f2deca) with main (13cac62)

Summary

✅ 20 untouched benchmarks

@Boshen
Copy link
Member

Boshen commented Nov 25, 2023

I found the test cases in test262. https://github.com/tc39/test262/blob/main/test/language/import/import-attributes/json-extensibility-array.js

Nice finding!

Let's update our submodule and remove this

"json-modules",

@Boshen
Copy link
Member

Boshen commented Nov 25, 2023

I found the test cases in test262. https://github.com/tc39/test262/blob/main/test/language/import/import-attributes/json-extensibility-array.js

Nice finding!

Let's update our submodule and remove this

"json-modules",

If I recall correctly, it was removed because it requires a resolver to statically check if the import is actually a json ... which is super weird for a parsing test.

Copy link
Member

@Boshen Boshen left a comment

Choose a reason for hiding this comment

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

Nice work! Thank you for bearing with me.

@Boshen Boshen merged commit 9ff0ffc into main Nov 25, 2023
17 checks passed
@Boshen Boshen deleted the ast/proposal-import-attributes branch November 25, 2023 07:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ast Area - AST A-codegen Area - Code Generation A-parser Area - Parser
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants