Implement Import Attributes proposal#4549
Conversation
Introduces the `ImportAttribute` struct, which represents
a single key-value pair in the import attributes syntax:
import json from "./foo.json" with { type: "json" };
The struct stores both the key and value as `Sym` (interned strings).
Extends `ImportDeclaration` to support import attributes by adding an `attributes` field that stores a boxed slice of `ImportAttribute`.
Extends the `ReExport` variant of `ExportDeclaration` to support import
attributes for re-export statements like:
export { val } from './foo.js' with { type: "json" };
export * from './foo.json' with { type: "json" };
Extends `ImportCall` to support the optional second argument used for
import attributes in dynamic imports:
import("foo.json", { with: { type: "json" } })
Implements parsing of the `with { ... }` clause used in import attributes:
import json from "./foo.json" with { type: "json" };
Updates the import declaration parser to parse import attributes using
the `WithClause` parser. Import declarations now support the syntax:
import json from "./foo.json" with { type: "json" };
import { val } from "./foo.js" with { type: "json" };
import * as ns from "./foo.json" with { type: "json" };
Updates the export declaration parser to parse import attributes for
re-export statements using the `WithClause` parser:
export * from './foo.json' with { type: "json" };
export * as ns from './foo.json' with { type: "json" };
export { val } from './foo.js' with { type: "json" };
Updates the dynamic import parser to handle the optional options argument
used for import attributes:
import("foo.json", { with: { type: "json" } })
* Implement dynamic import attributes support - Update Opcode::ImportCall to accept options operand. - Update ByteCompiler to emit ImportCall with options argument. - Update load_dyn_import to extract import attributes from options object. - Fix Instruction::ImportCall debug formatting. - Update ModuleLoader usage in tests. * Add test case for dynamic import with attributes * Refactor ModuleRequestsVisitor and fix clippy warnings * Remove unsafe_ignore_trace from ModuleRequest * Add test cases for static import and re-export with attributes * Fix test compilation by removing reference in assertion * Enforce string type for dynamic import attribute values - Update load_dyn_import to reject non-string values in import attributes. - Add test cases for invalid attribute value types. * cleanup
Test262 conformance changes
Fixed tests (89): |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4549 +/- ##
===========================================
+ Coverage 47.24% 57.40% +10.16%
===========================================
Files 476 504 +28
Lines 46892 57849 +10957
===========================================
+ Hits 22154 33209 +11055
+ Misses 24738 24640 -98 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
nekevss
left a comment
There was a problem hiding this comment.
There's still probably some more to review, but thought I'd leave some early review comments on this.
Overall, great work on this so far! Mostly had just some general nits and questions for now.
| matches!(tok.kind(), TokenKind::IdentifierName((sym, _)) if interner.resolve_expect(*sym).utf8().is_some_and(|s| s == "assert")) | ||
| }; | ||
|
|
||
| // Only treat `with` / `assert` as part of a with-clause if it is |
There was a problem hiding this comment.
question: why is assert still supported?
I only took a quick glance at the specification and it looks like assert was removed from the proposal.
There was a problem hiding this comment.
Oh, if you look at the bottom of the proposal, under history it says "For compatibility with existing implementations, the assert keyword will still be supported until it's safe to remove it, if it will ever be."
Though let me know if I’ve misunderstood though!
There was a problem hiding this comment.
Based off tc39/proposal-import-attributes#135, I would go with we should only support with out of the box.
There was a problem hiding this comment.
Thanks for the link, we've updated it to remove it entirely!
Co-authored-by: Haled Odat <8566042+HalidOdat@users.noreply.github.com>
Co-authored-by: Haled Odat <8566042+HalidOdat@users.noreply.github.com>
Co-authored-by: Haled Odat <8566042+HalidOdat@users.noreply.github.com>
HalidOdat
left a comment
There was a problem hiding this comment.
Awesome work! Looks good to me! 😄
nekevss
left a comment
There was a problem hiding this comment.
LGTM!
Thanks for adding all the tests for this as well 😄
|
Thanks for contributing this!!! |
This Pull Request closes #4440, implementing the Import Attributes proposal
It changes the following: