From 909963e82b5342426fea26cf93d7d819d089ef64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=B2=20Ribaudo?= Date: Tue, 12 Dec 2023 19:18:18 +0100 Subject: [PATCH 1/5] deps: V8: cherry-pick a0fd3209dda8 Original commit message: Reland "[import-attributes] Implement import attributes, with `assert` fallback" This is a reland of commit 159c82c5e6392e78b9bba7161b1bed6e23758984, to set the correct author. Original change's description: > [import-attributes] Implement import attributes, with `assert` fallback > > In the past six months, the old import assertions proposal has been > renamed to "import attributes" with the follwing major changes: > 1. the keyword is now `with` instead of `assert` > 2. unknown assertions cause an error rather than being ignored > > To preserve backward compatibility with existing applications that use > `assert`, implementations _can_ keep it around as a fallback for both > the static and dynamic forms. > > Additionally, the proposal has some minor changes that came up during > the stage 3 reviews: > 3. dynamic import first reads all the attributes, and then verifies > that they are all strings > 4. there is no need for a `[no LineTerminator here]` restriction before > the `with` keyword > 5. static import syntax allows any `LiteralPropertyName` as attribute > keys, to align with every other syntax using key-value pairs > > The new syntax is enabled by a new `--harmony-import-attributes` flag, > disabled by default. However, the new behavioral changes also apply to > the old syntax that is under the `--harmony-import-assertions` flag. > > This patch does implements (1), (3), (4) and (5). Handling of unknown > import assertions was not implemented directly in V8, but delegated > to embedders. As such, it will be implemented separately in d8 and > Chromium. > > To simplify the review, this patch doesn't migrate usage of the term > "assertions" to "attributes". There are many variables and internal > functions that could be easily renamed as soon as this patch landes. > There is one usage in the public API > (`ModuleRequest::GetImportAssertions`) that will probably need to be > aliased and then removed following the same process as for other API > breaking changes. > > Bug: v8:13856 > Change-Id: I78b167348d898887332c5ca7468bc5d58cd9b1ca > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4632799 > Commit-Queue: Shu-yu Guo > Reviewed-by: Adam Klein > Cr-Commit-Position: refs/heads/main@{#89110} Bug: v8:13856 Change-Id: Ic59aa3bd9101618e47ddf6cf6d6416a3a438ebec Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4705558 Commit-Queue: Shu-yu Guo Reviewed-by: Shu-yu Guo Reviewed-by: Adam Klein Cr-Commit-Position: refs/heads/main@{#89115} Refs: https://github.com/v8/v8/commit/a0fd3209dda8527d7da810abe231df27fffe789e --- deps/v8/include/v8-script.h | 7 +-- deps/v8/src/execution/isolate.cc | 47 ++++++++++---- deps/v8/src/flags/flag-definitions.h | 1 + deps/v8/src/init/bootstrapper.cc | 1 + deps/v8/src/init/heap-symbols.h | 1 + deps/v8/src/parsing/parser-base.h | 4 +- deps/v8/src/parsing/parser.cc | 43 ++++++++----- .../harmony/modules-import-attributes-1.mjs | 9 +++ .../harmony/modules-import-attributes-2.mjs | 9 +++ .../harmony/modules-import-attributes-3.mjs | 9 +++ .../harmony/modules-import-attributes-4.mjs | 9 +++ .../modules-import-attributes-dynamic-1.mjs | 12 ++++ .../modules-import-attributes-dynamic-10.mjs | 19 ++++++ .../modules-import-attributes-dynamic-11.mjs | 19 ++++++ .../modules-import-attributes-dynamic-12.mjs | 26 ++++++++ .../modules-import-attributes-dynamic-13.mjs | 26 ++++++++ .../modules-import-attributes-dynamic-2.mjs | 13 ++++ .../modules-import-attributes-dynamic-3.mjs | 13 ++++ .../modules-import-attributes-dynamic-4.mjs | 14 +++++ .../modules-import-attributes-dynamic-5.mjs | 12 ++++ .../modules-import-attributes-dynamic-6.mjs | 18 ++++++ .../modules-import-attributes-dynamic-7.mjs | 63 +++++++++++++++++++ .../modules-import-attributes-dynamic-8.mjs | 13 ++++ .../modules-import-attributes-dynamic-9.mjs | 13 ++++ ...tributes-dynamic-assertions-fallback-1.mjs | 15 +++++ ...tributes-dynamic-assertions-fallback-2.mjs | 15 +++++ 26 files changed, 398 insertions(+), 33 deletions(-) create mode 100644 deps/v8/test/mjsunit/harmony/modules-import-attributes-1.mjs create mode 100644 deps/v8/test/mjsunit/harmony/modules-import-attributes-2.mjs create mode 100644 deps/v8/test/mjsunit/harmony/modules-import-attributes-3.mjs create mode 100644 deps/v8/test/mjsunit/harmony/modules-import-attributes-4.mjs create mode 100644 deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-1.mjs create mode 100644 deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-10.mjs create mode 100644 deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-11.mjs create mode 100644 deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-12.mjs create mode 100644 deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-13.mjs create mode 100644 deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-2.mjs create mode 100644 deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-3.mjs create mode 100644 deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-4.mjs create mode 100644 deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-5.mjs create mode 100644 deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-6.mjs create mode 100644 deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-7.mjs create mode 100644 deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-8.mjs create mode 100644 deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-9.mjs create mode 100644 deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-assertions-fallback-1.mjs create mode 100644 deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-assertions-fallback-2.mjs diff --git a/deps/v8/include/v8-script.h b/deps/v8/include/v8-script.h index 5644a3bb70c6..91b0a3d3e733 100644 --- a/deps/v8/include/v8-script.h +++ b/deps/v8/include/v8-script.h @@ -128,10 +128,9 @@ class V8_EXPORT ModuleRequest : public Data { * * All assertions present in the module request will be supplied in this * list, regardless of whether they are supported by the host. Per - * https://tc39.es/proposal-import-assertions/#sec-hostgetsupportedimportassertions, - * hosts are expected to ignore assertions that they do not support (as - * opposed to, for example, triggering an error if an unsupported assertion is - * present). + * https://tc39.es/proposal-import-attributes/#sec-hostgetsupportedimportattributes, + * hosts are expected to throw for assertions that they do not support (as + * opposed to, for example, ignoring them). */ Local GetImportAssertions() const; diff --git a/deps/v8/src/execution/isolate.cc b/deps/v8/src/execution/isolate.cc index e5dba75764f6..d9bb966fe64a 100644 --- a/deps/v8/src/execution/isolate.cc +++ b/deps/v8/src/execution/isolate.cc @@ -4726,7 +4726,7 @@ MaybeHandle Isolate::GetImportAssertionsFromArgument( // The parser shouldn't have allowed the second argument to import() if // the flag wasn't enabled. - DCHECK(FLAG_harmony_import_assertions); + DCHECK(FLAG_harmony_import_assertions || FLAG_harmony_import_attributes); if (!import_assertions_argument->IsJSReceiver()) { this->Throw( @@ -4736,18 +4736,35 @@ MaybeHandle Isolate::GetImportAssertionsFromArgument( Handle import_assertions_argument_receiver = Handle::cast(import_assertions_argument); - Handle key = factory()->assert_string(); Handle import_assertions_object; - if (!JSReceiver::GetProperty(this, import_assertions_argument_receiver, key) - .ToHandle(&import_assertions_object)) { - // This can happen if the property has a getter function that throws - // an error. - return MaybeHandle(); + + if (FLAG_harmony_import_attributes) { + Handle with_key = factory()->with_string(); + if (!JSReceiver::GetProperty(this, import_assertions_argument_receiver, + with_key) + .ToHandle(&import_assertions_object)) { + // This can happen if the property has a getter function that throws + // an error. + return MaybeHandle(); + } } - // If there is no 'assert' option in the options bag, it's not an error. Just - // do the import() as if no assertions were provided. + if (FLAG_harmony_import_assertions && + (!FLAG_harmony_import_attributes || + import_assertions_object->IsUndefined())) { + Handle assert_key = factory()->assert_string(); + if (!JSReceiver::GetProperty(this, import_assertions_argument_receiver, + assert_key) + .ToHandle(&import_assertions_object)) { + // This can happen if the property has a getter function that throws + // an error. + return MaybeHandle(); + } + } + + // If there is no 'with' or 'assert' option in the options bag, it's not an + // error. Just do the import() as if no assertions were provided. if (import_assertions_object->IsUndefined()) return import_assertions_array; if (!import_assertions_object->IsJSReceiver()) { @@ -4769,6 +4786,8 @@ MaybeHandle Isolate::GetImportAssertionsFromArgument( return MaybeHandle(); } + bool has_non_string_attribute = false; + // The assertions will be passed to the host in the form: [key1, // value1, key2, value2, ...]. constexpr size_t kAssertionEntrySizeForDynamicImport = 2; @@ -4786,9 +4805,7 @@ MaybeHandle Isolate::GetImportAssertionsFromArgument( } if (!assertion_value->IsString()) { - this->Throw(*factory()->NewTypeError( - MessageTemplate::kNonStringImportAssertionValue)); - return MaybeHandle(); + has_non_string_attribute = true; } import_assertions_array->set((i * kAssertionEntrySizeForDynamicImport), @@ -4797,6 +4814,12 @@ MaybeHandle Isolate::GetImportAssertionsFromArgument( *assertion_value); } + if (has_non_string_attribute) { + this->Throw(*factory()->NewTypeError( + MessageTemplate::kNonStringImportAssertionValue)); + return MaybeHandle(); + } + return import_assertions_array; } diff --git a/deps/v8/src/flags/flag-definitions.h b/deps/v8/src/flags/flag-definitions.h index f02ef309d69e..48dc37118c54 100644 --- a/deps/v8/src/flags/flag-definitions.h +++ b/deps/v8/src/flags/flag-definitions.h @@ -300,6 +300,7 @@ DEFINE_BOOL(harmony_shipping, true, "enable all shipped harmony features") // Features that are still work in progress (behind individual flags). #define HARMONY_INPROGRESS_BASE(V) \ + V(harmony_import_attributes, "harmony import attributes") \ V(harmony_weak_refs_with_cleanup_some, \ "harmony weak references with FinalizationRegistry.prototype.cleanupSome") \ V(harmony_import_assertions, "harmony import assertions") \ diff --git a/deps/v8/src/init/bootstrapper.cc b/deps/v8/src/init/bootstrapper.cc index d4c4122c22f4..ead61d622290 100644 --- a/deps/v8/src/init/bootstrapper.cc +++ b/deps/v8/src/init/bootstrapper.cc @@ -4482,6 +4482,7 @@ void Genesis::InitializeConsole(Handle extras_binding) { void Genesis::InitializeGlobal_##id() {} EMPTY_INITIALIZE_GLOBAL_FOR_FEATURE(harmony_import_assertions) +EMPTY_INITIALIZE_GLOBAL_FOR_FEATURE(harmony_import_attributes) EMPTY_INITIALIZE_GLOBAL_FOR_FEATURE(harmony_private_brand_checks) EMPTY_INITIALIZE_GLOBAL_FOR_FEATURE(harmony_class_static_blocks) EMPTY_INITIALIZE_GLOBAL_FOR_FEATURE(harmony_error_cause) diff --git a/deps/v8/src/init/heap-symbols.h b/deps/v8/src/init/heap-symbols.h index 8cd69613a678..cfff76beb4aa 100644 --- a/deps/v8/src/init/heap-symbols.h +++ b/deps/v8/src/init/heap-symbols.h @@ -432,6 +432,7 @@ V(_, week_string, "week") \ V(_, weeks_string, "weeks") \ V(_, weekOfYear_string, "weekOfYear") \ + V(_, with_string, "with") \ V(_, word_string, "word") \ V(_, writable_string, "writable") \ V(_, yearMonthFromFields_string, "yearMonthFromFields") \ diff --git a/deps/v8/src/parsing/parser-base.h b/deps/v8/src/parsing/parser-base.h index cfa91e446f82..30dff440f652 100644 --- a/deps/v8/src/parsing/parser-base.h +++ b/deps/v8/src/parsing/parser-base.h @@ -3725,7 +3725,9 @@ ParserBase::ParseImportExpressions() { AcceptINScope scope(this, true); ExpressionT specifier = ParseAssignmentExpressionCoverGrammar(); - if (FLAG_harmony_import_assertions && Check(Token::COMMA)) { + if ((FLAG_harmony_import_assertions || + FLAG_harmony_import_attributes) && + Check(Token::COMMA)) { if (Check(Token::RPAREN)) { // A trailing comma allowed after the specifier. return factory()->NewImportCallExpression(specifier, pos); diff --git a/deps/v8/src/parsing/parser.cc b/deps/v8/src/parsing/parser.cc index 9789909456ee..92beebba5233 100644 --- a/deps/v8/src/parsing/parser.cc +++ b/deps/v8/src/parsing/parser.cc @@ -1344,27 +1344,34 @@ ZonePtrList* Parser::ParseNamedImports(int pos) { } ImportAssertions* Parser::ParseImportAssertClause() { - // AssertClause : - // assert '{' '}' - // assert '{' AssertEntries '}' + // WithClause : + // with '{' '}' + // with '{' WithEntries ','? '}' - // AssertEntries : - // IdentifierName: AssertionKey - // IdentifierName: AssertionKey , AssertEntries + // WithEntries : + // LiteralPropertyName + // LiteralPropertyName ':' StringLiteral , WithEntries - // AssertionKey : - // IdentifierName - // StringLiteral + // (DEPRECATED) + // AssertClause : + // assert '{' '}' + // assert '{' WithEntries ','? '}' auto import_assertions = zone()->New(zone()); - if (!FLAG_harmony_import_assertions) { - return import_assertions; - } + if (FLAG_harmony_import_attributes && Check(Token::WITH)) { + // 'with' keyword consumed + } else if (FLAG_harmony_import_assertions && + !scanner()->HasLineTerminatorBeforeNext() && + CheckContextualKeyword(ast_value_factory()->assert_string())) { + // The 'assert' contextual keyword is deprecated in favor of 'with', and we + // need to investigate feasibility of unshipping. + // + // TODO(v8:13856): Remove once decision is made to unship 'assert' or keep. - // Assert clause is optional, and cannot be preceded by a LineTerminator. - if (scanner()->HasLineTerminatorBeforeNext() || - !CheckContextualKeyword(ast_value_factory()->assert_string())) { + // NOTE(Node.js): Commented out to avoid backporting this use counter to Node.js 18 + // ++use_counts_[v8::Isolate::kImportAssertionDeprecatedSyntax]; + } else { return import_assertions; } @@ -1372,8 +1379,12 @@ ImportAssertions* Parser::ParseImportAssertClause() { while (peek() != Token::RBRACE) { const AstRawString* attribute_key = nullptr; - if (Check(Token::STRING)) { + if (Check(Token::STRING) || Check(Token::SMI)) { attribute_key = GetSymbol(); + } else if (Check(Token::NUMBER)) { + attribute_key = GetNumberAsSymbol(); + } else if (Check(Token::BIGINT)) { + attribute_key = GetBigIntAsSymbol(); } else { attribute_key = ParsePropertyName(); } diff --git a/deps/v8/test/mjsunit/harmony/modules-import-attributes-1.mjs b/deps/v8/test/mjsunit/harmony/modules-import-attributes-1.mjs new file mode 100644 index 000000000000..3cf6bac870b8 --- /dev/null +++ b/deps/v8/test/mjsunit/harmony/modules-import-attributes-1.mjs @@ -0,0 +1,9 @@ +// Copyright 2021 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --harmony-import-attributes + +import { life } from 'modules-skip-1.mjs' with { }; + +assertEquals(42, life()); diff --git a/deps/v8/test/mjsunit/harmony/modules-import-attributes-2.mjs b/deps/v8/test/mjsunit/harmony/modules-import-attributes-2.mjs new file mode 100644 index 000000000000..63f5859ca0f1 --- /dev/null +++ b/deps/v8/test/mjsunit/harmony/modules-import-attributes-2.mjs @@ -0,0 +1,9 @@ +// Copyright 2021 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --harmony-import-attributes + +import json from 'modules-skip-1.json' with { type: 'json' }; + +assertEquals(42, json.life); diff --git a/deps/v8/test/mjsunit/harmony/modules-import-attributes-3.mjs b/deps/v8/test/mjsunit/harmony/modules-import-attributes-3.mjs new file mode 100644 index 000000000000..34db1852367c --- /dev/null +++ b/deps/v8/test/mjsunit/harmony/modules-import-attributes-3.mjs @@ -0,0 +1,9 @@ +// Copyright 2021 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --harmony-import-attributes + +import {life} from 'modules-skip-imports-json-1.mjs'; + +assertEquals(42, life()); diff --git a/deps/v8/test/mjsunit/harmony/modules-import-attributes-4.mjs b/deps/v8/test/mjsunit/harmony/modules-import-attributes-4.mjs new file mode 100644 index 000000000000..66bdc11c359e --- /dev/null +++ b/deps/v8/test/mjsunit/harmony/modules-import-attributes-4.mjs @@ -0,0 +1,9 @@ +// Copyright 2021 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --harmony-import-attributes + +import json from 'modules-skip-1.json' with { type: 'json', notARealAssertion: 'value'}; + +assertEquals(42, json.life); diff --git a/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-1.mjs b/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-1.mjs new file mode 100644 index 000000000000..644fc96a6e21 --- /dev/null +++ b/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-1.mjs @@ -0,0 +1,12 @@ +// Copyright 2021 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax --harmony-import-attributes + +var life; +import('modules-skip-1.mjs', { }).then(namespace => life = namespace.life()); + +%PerformMicrotaskCheckpoint(); + +assertEquals(42, life); diff --git a/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-10.mjs b/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-10.mjs new file mode 100644 index 000000000000..972fbed24bff --- /dev/null +++ b/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-10.mjs @@ -0,0 +1,19 @@ +// Copyright 2021 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax --harmony-import-attributes + +var result1; +var result2; +import('modules-skip-1.json', { get with() { throw 'bad \'with\' getter!'; } }).then( + () => assertUnreachable('Should have failed due to throwing getter'), + error => result1 = error); +import('modules-skip-1.json', { with: { get assertionKey() { throw 'bad \'assertionKey\' getter!'; } } }).then( + () => assertUnreachable('Should have failed due to throwing getter'), + error => result2 = error); + +%PerformMicrotaskCheckpoint(); + +assertEquals('bad \'with\' getter!', result1); +assertEquals('bad \'assertionKey\' getter!', result2); \ No newline at end of file diff --git a/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-11.mjs b/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-11.mjs new file mode 100644 index 000000000000..c39a5f9d4e2e --- /dev/null +++ b/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-11.mjs @@ -0,0 +1,19 @@ +// Copyright 2021 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax --harmony-import-attributes --harmony-top-level-await + +var life1; +var life2; +import('modules-skip-1.json', { with: { type: 'json' } }).then( + namespace => life1 = namespace.default.life); + +// Try loading the same module a second time. +import('modules-skip-1.json', { with: { type: 'json' } }).then( + namespace => life2 = namespace.default.life); + +%PerformMicrotaskCheckpoint(); + +assertEquals(42, life1); +assertEquals(42, life2); diff --git a/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-12.mjs b/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-12.mjs new file mode 100644 index 000000000000..1d8fb21a5022 --- /dev/null +++ b/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-12.mjs @@ -0,0 +1,26 @@ +// Copyright 2021 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax --harmony-import-attributes + +let result1; +let result2; + +let badAssertProxy1 = new Proxy({}, { ownKeys() { throw "bad ownKeys!"; } }); +import('./modules-skip-1.mjs', { with: badAssertProxy1 }).then( + () => assertUnreachable('Should have failed due to throwing ownKeys'), + error => result1 = error); + +let badAssertProxy2 = new Proxy( + {foo: "bar"}, + { getOwnPropertyDescriptor() { throw "bad getOwnPropertyDescriptor!"; } }); +import('./modules-skip-1.mjs', { with: badAssertProxy2 }).then( + () => assertUnreachable( + 'Should have failed due to throwing getOwnPropertyDescriptor'), + error => result2 = error); + +%PerformMicrotaskCheckpoint(); + +assertEquals('bad ownKeys!', result1); +assertEquals('bad getOwnPropertyDescriptor!', result2); \ No newline at end of file diff --git a/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-13.mjs b/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-13.mjs new file mode 100644 index 000000000000..6e23c07c281d --- /dev/null +++ b/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-13.mjs @@ -0,0 +1,26 @@ +// Copyright 2021 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax --harmony-import-attributes + +let getters = 0; +let result; + +import('./modules-skip-1.mjs', { with: { + get attr1() { + getters++; + return {}; + }, + get attr2() { + getters++; + return {}; + }, +} }).then( + () => assertUnreachable('Should have failed due to invalid attributes values'), + error => result = error); + +%PerformMicrotaskCheckpoint(); + +assertEquals(2, getters); +assertInstanceof(result, TypeError); diff --git a/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-2.mjs b/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-2.mjs new file mode 100644 index 000000000000..b8e59fb0bfe9 --- /dev/null +++ b/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-2.mjs @@ -0,0 +1,13 @@ +// Copyright 2021 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax --harmony-import-attributes + +var life; +import('modules-skip-1.mjs', { with: { } }).then( + namespace => life = namespace.life()); + +%PerformMicrotaskCheckpoint(); + +assertEquals(42, life); diff --git a/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-3.mjs b/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-3.mjs new file mode 100644 index 000000000000..da570eb2836e --- /dev/null +++ b/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-3.mjs @@ -0,0 +1,13 @@ +// Copyright 2021 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax --harmony-import-attributes + +var life; +import('modules-skip-1.json', { with: { type: 'json' } }).then( + namespace => life = namespace.default.life); + +%PerformMicrotaskCheckpoint(); + +assertEquals(42, life); diff --git a/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-4.mjs b/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-4.mjs new file mode 100644 index 000000000000..fff6fe2bbef8 --- /dev/null +++ b/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-4.mjs @@ -0,0 +1,14 @@ +// Copyright 2021 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax --harmony-import-attributes + +var result; +import('modules-skip-1.json', { with: { type: 'notARealType' } }).then( + () => assertUnreachable('Should have failed due to bad module type'), + error => result = error.message); + +%PerformMicrotaskCheckpoint(); + +assertEquals('Invalid module type was asserted', result); diff --git a/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-5.mjs b/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-5.mjs new file mode 100644 index 000000000000..cdb1567c3307 --- /dev/null +++ b/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-5.mjs @@ -0,0 +1,12 @@ +// Copyright 2021 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax --harmony-import-attributes + +var life; +import('modules-skip-imports-json-1.mjs',).then(namespace => life = namespace.life()); + +%PerformMicrotaskCheckpoint(); + +assertEquals(42, life); diff --git a/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-6.mjs b/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-6.mjs new file mode 100644 index 000000000000..ad84e3edd5ff --- /dev/null +++ b/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-6.mjs @@ -0,0 +1,18 @@ +// Copyright 2021 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax --harmony-import-attributes + +var life; +import('modules-skip-1.json', { with: { type: 'json', notARealAssertion: 'value' } }).then( + namespace => life = namespace.default.life); + +var life2; +import('modules-skip-1.json', { with: { 0: 'value', type: 'json' } }).then( + namespace => life2 = namespace.default.life); + +%PerformMicrotaskCheckpoint(); + +assertEquals(42, life); +assertEquals(42, life2); diff --git a/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-7.mjs b/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-7.mjs new file mode 100644 index 000000000000..530d833cce5a --- /dev/null +++ b/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-7.mjs @@ -0,0 +1,63 @@ +// Copyright 2021 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax --harmony-import-attributes + +var result1; +var result2; +var result3; +var result4; +var result5; +var result6; +var result7; +var result8; +var result9; +var result10; +import('modules-skip-1.json', null).then( + () => assertUnreachable('Should have failed due to non-object parameter'), + error => result1 = error.message); +import('modules-skip-1.json', 7).then( + () => assertUnreachable('Should have failed due to non-object parameter'), + error => result2 = error.message); +import('modules-skip-1.json', 'string').then( + () => assertUnreachable('Should have failed due to non-object parameter'), + error => result3 = error.message); +import('modules-skip-1.json', { with: null}).then( + () => assertUnreachable('Should have failed due to bad with object'), + error => result4 = error.message); +import('modules-skip-1.json', { with: 7}).then( + () => assertUnreachable('Should have failed due to bad with object'), + error => result5 = error.message); +import('modules-skip-1.json', { with: 'string'}).then( + () => assertUnreachable('Should have failed due to bad with object'), + error => result6 = error.message); +import('modules-skip-1.json', { with: { a: null }}).then( + () => assertUnreachable('Should have failed due to bad with object'), + error => result7 = error.message); +import('modules-skip-1.json', { with: { a: undefined }}).then( + () => assertUnreachable('Should have failed due to bad assertion value'), + error => result8 = error.message); +import('modules-skip-1.json', { with: { a: 7 }}).then( + () => assertUnreachable('Should have failed due to bad assertion value'), + error => result9 = error.message); + import('modules-skip-1.json', { with: { a: { } }}).then( + () => assertUnreachable('Should have failed due to bad assertion value'), + error => result10 = error.message); + +%PerformMicrotaskCheckpoint(); + +const argumentNotObjectError = 'The second argument to import() must be an object'; +const assertOptionNotObjectError = 'The \'assert\' option must be an object'; +const assertionValueNotStringError = 'Import assertion value must be a string'; + +assertEquals(argumentNotObjectError, result1); +assertEquals(argumentNotObjectError, result2); +assertEquals(argumentNotObjectError, result3); +assertEquals(assertOptionNotObjectError, result4); +assertEquals(assertOptionNotObjectError, result5); +assertEquals(assertOptionNotObjectError, result6); +assertEquals(assertionValueNotStringError, result7); +assertEquals(assertionValueNotStringError, result8); +assertEquals(assertionValueNotStringError, result9); +assertEquals(assertionValueNotStringError, result10); diff --git a/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-8.mjs b/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-8.mjs new file mode 100644 index 000000000000..74a4504e253d --- /dev/null +++ b/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-8.mjs @@ -0,0 +1,13 @@ +// Copyright 2021 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax --harmony-import-attributes + +var life; +import('modules-skip-1.mjs', undefined).then( + namespace => life = namespace.life()); + +%PerformMicrotaskCheckpoint(); + +assertEquals(42, life); diff --git a/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-9.mjs b/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-9.mjs new file mode 100644 index 000000000000..1b56c70df616 --- /dev/null +++ b/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-9.mjs @@ -0,0 +1,13 @@ +// Copyright 2021 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax --harmony-import-attributes + +var life; +import('modules-skip-1.mjs', { with: undefined }).then( + namespace => life = namespace.life()); + +%PerformMicrotaskCheckpoint(); + +assertEquals(42, life); diff --git a/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-assertions-fallback-1.mjs b/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-assertions-fallback-1.mjs new file mode 100644 index 000000000000..3faaf0dccea1 --- /dev/null +++ b/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-assertions-fallback-1.mjs @@ -0,0 +1,15 @@ +// Copyright 2021 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax --harmony-import-attributes --harmony-import-assertions + +var life; +import('modules-skip-1.mjs', { + with: {}, + get assert() { throw 'Should not read assert'; } +}).then(namespace => life = namespace.life()); + +%PerformMicrotaskCheckpoint(); + +assertEquals(42, life); diff --git a/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-assertions-fallback-2.mjs b/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-assertions-fallback-2.mjs new file mode 100644 index 000000000000..4c83d2fbd716 --- /dev/null +++ b/deps/v8/test/mjsunit/harmony/modules-import-attributes-dynamic-assertions-fallback-2.mjs @@ -0,0 +1,15 @@ +// Copyright 2021 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax --harmony-import-attributes --harmony-import-assertions + +var life; +import('modules-skip-1.json', { + with: undefined, + assert: { type: 'json' } +}).then(namespace => life = namespace.default.life); + +%PerformMicrotaskCheckpoint(); + +assertEquals(42, life); From 5389c3a4e22211a9e69fda10db1a3e9c90f094b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=B2=20Ribaudo?= Date: Tue, 12 Dec 2023 19:18:24 +0100 Subject: [PATCH 2/5] deps: V8: cherry-pick ea996ad04a68 Original commit message: [import-attributes] Remove support for numeric keys During the 2023-09 TC39 meeting the proposal has been updated to remove support for bigint and float literals as import attribute keys, due to implementation difficulties in other engines and minimal added value for JS developers. GH issue: https://github.com/tc39/proposal-import-attributes/issues/145 Bug: v8:13856 Change-Id: I0ede2bb10d6ca338a4b0870a1261ccbcd088c16f Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4899760 Reviewed-by: Shu-yu Guo Commit-Queue: Joyee Cheung Cr-Commit-Position: refs/heads/main@{#90318} Refs: https://github.com/v8/v8/commit/ea996ad04a684e32cd93018f0aad2f46e6b86975 --- deps/v8/src/parsing/parser.cc | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/deps/v8/src/parsing/parser.cc b/deps/v8/src/parsing/parser.cc index 92beebba5233..4a60275a546d 100644 --- a/deps/v8/src/parsing/parser.cc +++ b/deps/v8/src/parsing/parser.cc @@ -1378,16 +1378,8 @@ ImportAssertions* Parser::ParseImportAssertClause() { Expect(Token::LBRACE); while (peek() != Token::RBRACE) { - const AstRawString* attribute_key = nullptr; - if (Check(Token::STRING) || Check(Token::SMI)) { - attribute_key = GetSymbol(); - } else if (Check(Token::NUMBER)) { - attribute_key = GetNumberAsSymbol(); - } else if (Check(Token::BIGINT)) { - attribute_key = GetBigIntAsSymbol(); - } else { - attribute_key = ParsePropertyName(); - } + const AstRawString* attribute_key = + Check(Token::STRING) ? GetSymbol() : ParsePropertyName(); Scanner::Location location = scanner()->location(); From b9d4d7cbad1077356841916f583f581299e04ef9 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Tue, 12 Dec 2023 19:18:28 +0100 Subject: [PATCH 3/5] esm: use import attributes instead of import assertions The old import assertions proposal has been renamed to "import attributes" with the follwing major changes: 1. The keyword is now `with` instead of `assert`. 2. Unknown assertions cause an error rather than being ignored, This commit updates the documentation to encourage folks to use the new syntax, and add aliases for module customization hooks. PR-URL: https://github.com/nodejs/node/pull/50140 Fixes: https://github.com/nodejs/node/issues/50134 Refs: https://github.com/v8/v8/commit/159c82c5e6392e78b9bba7161b1bed6e23758984 Reviewed-By: Geoffrey Booth Reviewed-By: Jacob Smith Reviewed-By: Benjamin Gruenbaum --- .eslintrc.js | 5 +---- doc/api/esm.md | 20 ++++++++++--------- src/node.cc | 7 +++++++ .../test-esm-assertionless-json-import.js | 10 +++++----- test/es-module/test-esm-data-urls.js | 10 +++++----- .../test-esm-dynamic-import-attribute.js | 8 ++++---- .../test-esm-dynamic-import-attribute.mjs | 8 ++++---- .../test-esm-import-attributes-1.mjs | 2 +- .../test-esm-import-attributes-2.mjs | 4 ++-- .../test-esm-import-attributes-3.mjs | 4 ++-- .../test-esm-import-attributes-errors.js | 16 +++++++-------- .../test-esm-import-attributes-errors.mjs | 14 ++++++------- test/es-module/test-esm-json-cache.mjs | 2 +- test/es-module/test-esm-json.mjs | 10 +++++----- test/es-module/test-esm-virtual-json.mjs | 4 ++-- .../es-modules/import-json-named-export.mjs | 2 +- test/fixtures/es-modules/json-modules.mjs | 2 +- .../parallel/test-vm-module-dynamic-import.js | 2 +- 18 files changed, 68 insertions(+), 62 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index b427c9986f67..49a810249146 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -44,10 +44,7 @@ module.exports = { parserOptions: { babelOptions: { plugins: [ - [ - Module._findPath('@babel/plugin-syntax-import-attributes'), - { deprecatedAssertSyntax: true }, - ], + Module._findPath('@babel/plugin-syntax-import-attributes'), ], }, requireConfigFile: false, diff --git a/doc/api/esm.md b/doc/api/esm.md index 9b629f2a2a99..448a3c119513 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -7,6 +7,9 @@ @@ -251,18 +254,17 @@ changes: > Stability: 1.1 - Active development > This feature was previously named "Import assertions", and using the `assert` -> keyword instead of `with`. Because the version of V8 on this release line does -> not support the `with` keyword, you need to keep using `assert` to support -> this version of Node.js. +> keyword instead of `with`. Any uses in code of the prior `assert` keyword +> should be updated to use `with` instead. The [Import Attributes proposal][] adds an inline syntax for module import statements to pass on more information alongside the module specifier. ```js -import fooData from './foo.json' assert { type: 'json' }; +import fooData from './foo.json' with { type: 'json' }; const { default: barData } = - await import('./bar.json', { assert: { type: 'json' } }); + await import('./bar.json', { with: { type: 'json' } }); ``` Node.js supports the following `type` values, for which the attribute is @@ -555,10 +557,10 @@ separate cache. JSON files can be referenced by `import`: ```js -import packageConfig from './package.json' assert { type: 'json' }; +import packageConfig from './package.json' with { type: 'json' }; ``` -The `assert { type: 'json' }` syntax is mandatory; see [Import Attributes][]. +The `with { type: 'json' }` syntax is mandatory; see [Import Attributes][]. The imported JSON only exposes a `default` export. There is no support for named exports. A cache entry is created in the CommonJS cache to avoid duplication. diff --git a/src/node.cc b/src/node.cc index 0c75b5a08edd..367d47325b7d 100644 --- a/src/node.cc +++ b/src/node.cc @@ -745,6 +745,13 @@ int ProcessGlobalArgs(std::vector* args, "--no-harmony-import-assertions") == v8_args.end()) { v8_args.emplace_back("--harmony-import-assertions"); } + // TODO(aduh95): remove this when the harmony-import-attributes flag + // is removed in V8. + if (std::find(v8_args.begin(), + v8_args.end(), + "--no-harmony-import-attributes") == v8_args.end()) { + v8_args.emplace_back("--harmony-import-attributes"); + } auto env_opts = per_process::cli_options->per_isolate->per_env; if (std::find(v8_args.begin(), v8_args.end(), diff --git a/test/es-module/test-esm-assertionless-json-import.js b/test/es-module/test-esm-assertionless-json-import.js index 23c71a1ba105..bd8acdae995a 100644 --- a/test/es-module/test-esm-assertionless-json-import.js +++ b/test/es-module/test-esm-assertionless-json-import.js @@ -9,7 +9,7 @@ async function test() { import('../fixtures/experimental.json'), import( '../fixtures/experimental.json', - { assert: { type: 'json' } } + { with: { type: 'json' } } ), ]); @@ -24,7 +24,7 @@ async function test() { import('../fixtures/experimental.json?test'), import( '../fixtures/experimental.json?test', - { assert: { type: 'json' } } + { with: { type: 'json' } } ), ]); @@ -39,7 +39,7 @@ async function test() { import('../fixtures/experimental.json#test'), import( '../fixtures/experimental.json#test', - { assert: { type: 'json' } } + { with: { type: 'json' } } ), ]); @@ -54,7 +54,7 @@ async function test() { import('../fixtures/experimental.json?test2#test'), import( '../fixtures/experimental.json?test2#test', - { assert: { type: 'json' } } + { with: { type: 'json' } } ), ]); @@ -69,7 +69,7 @@ async function test() { import('data:application/json,{"ofLife":42}'), import( 'data:application/json,{"ofLife":42}', - { assert: { type: 'json' } } + { with: { type: 'json' } } ), ]); diff --git a/test/es-module/test-esm-data-urls.js b/test/es-module/test-esm-data-urls.js index 5be45d0f7af3..a0add97b6f64 100644 --- a/test/es-module/test-esm-data-urls.js +++ b/test/es-module/test-esm-data-urls.js @@ -60,21 +60,21 @@ function createBase64URL(mime, body) { } { const ns = await import('data:application/json;foo="test,"this"', - { assert: { type: 'json' } }); + { with: { type: 'json' } }); assert.deepStrictEqual(Object.keys(ns), ['default']); assert.strictEqual(ns.default, 'this'); } { const ns = await import(`data:application/json;foo=${ encodeURIComponent('test,') - },0`, { assert: { type: 'json' } }); + },0`, { with: { type: 'json' } }); assert.deepStrictEqual(Object.keys(ns), ['default']); assert.strictEqual(ns.default, 0); } { await assert.rejects(async () => import('data:application/json;foo="test,",0', - { assert: { type: 'json' } }), { + { with: { type: 'json' } }), { name: 'SyntaxError', message: /Unexpected end of JSON input/ }); @@ -82,14 +82,14 @@ function createBase64URL(mime, body) { { const body = '{"x": 1}'; const plainESMURL = createURL('application/json', body); - const ns = await import(plainESMURL, { assert: { type: 'json' } }); + const ns = await import(plainESMURL, { with: { type: 'json' } }); assert.deepStrictEqual(Object.keys(ns), ['default']); assert.strictEqual(ns.default.x, 1); } { const body = '{"default": 2}'; const plainESMURL = createURL('application/json', body); - const ns = await import(plainESMURL, { assert: { type: 'json' } }); + const ns = await import(plainESMURL, { with: { type: 'json' } }); assert.deepStrictEqual(Object.keys(ns), ['default']); assert.strictEqual(ns.default.default, 2); } diff --git a/test/es-module/test-esm-dynamic-import-attribute.js b/test/es-module/test-esm-dynamic-import-attribute.js index 71ef9cd1d1d3..4558cd27ca42 100644 --- a/test/es-module/test-esm-dynamic-import-attribute.js +++ b/test/es-module/test-esm-dynamic-import-attribute.js @@ -5,7 +5,7 @@ const { strictEqual } = require('assert'); async function test() { { const results = await Promise.allSettled([ - import('../fixtures/empty.js', { assert: { type: 'json' } }), + import('../fixtures/empty.js', { with: { type: 'json' } }), import('../fixtures/empty.js'), ]); @@ -16,7 +16,7 @@ async function test() { { const results = await Promise.allSettled([ import('../fixtures/empty.js'), - import('../fixtures/empty.js', { assert: { type: 'json' } }), + import('../fixtures/empty.js', { with: { type: 'json' } }), ]); strictEqual(results[0].status, 'fulfilled'); @@ -25,7 +25,7 @@ async function test() { { const results = await Promise.allSettled([ - import('../fixtures/empty.json', { assert: { type: 'json' } }), + import('../fixtures/empty.json', { with: { type: 'json' } }), import('../fixtures/empty.json'), ]); @@ -36,7 +36,7 @@ async function test() { { const results = await Promise.allSettled([ import('../fixtures/empty.json'), - import('../fixtures/empty.json', { assert: { type: 'json' } }), + import('../fixtures/empty.json', { with: { type: 'json' } }), ]); strictEqual(results[0].status, 'rejected'); diff --git a/test/es-module/test-esm-dynamic-import-attribute.mjs b/test/es-module/test-esm-dynamic-import-attribute.mjs index 4010259b743c..b3d2cb20c36e 100644 --- a/test/es-module/test-esm-dynamic-import-attribute.mjs +++ b/test/es-module/test-esm-dynamic-import-attribute.mjs @@ -3,7 +3,7 @@ import { strictEqual } from 'assert'; { const results = await Promise.allSettled([ - import('../fixtures/empty.js', { assert: { type: 'json' } }), + import('../fixtures/empty.js', { with: { type: 'json' } }), import('../fixtures/empty.js'), ]); @@ -14,7 +14,7 @@ import { strictEqual } from 'assert'; { const results = await Promise.allSettled([ import('../fixtures/empty.js'), - import('../fixtures/empty.js', { assert: { type: 'json' } }), + import('../fixtures/empty.js', { with: { type: 'json' } }), ]); strictEqual(results[0].status, 'fulfilled'); @@ -23,7 +23,7 @@ import { strictEqual } from 'assert'; { const results = await Promise.allSettled([ - import('../fixtures/empty.json', { assert: { type: 'json' } }), + import('../fixtures/empty.json', { with: { type: 'json' } }), import('../fixtures/empty.json'), ]); @@ -34,7 +34,7 @@ import { strictEqual } from 'assert'; { const results = await Promise.allSettled([ import('../fixtures/empty.json'), - import('../fixtures/empty.json', { assert: { type: 'json' } }), + import('../fixtures/empty.json', { with: { type: 'json' } }), ]); strictEqual(results[0].status, 'rejected'); diff --git a/test/es-module/test-esm-import-attributes-1.mjs b/test/es-module/test-esm-import-attributes-1.mjs index 72b3426bdbb6..c699a27bb51a 100644 --- a/test/es-module/test-esm-import-attributes-1.mjs +++ b/test/es-module/test-esm-import-attributes-1.mjs @@ -1,6 +1,6 @@ import '../common/index.mjs'; import { strictEqual } from 'assert'; -import secret from '../fixtures/experimental.json' assert { type: 'json' }; +import secret from '../fixtures/experimental.json' with { type: 'json' }; strictEqual(secret.ofLife, 42); diff --git a/test/es-module/test-esm-import-attributes-2.mjs b/test/es-module/test-esm-import-attributes-2.mjs index 1b4669ac2764..85d6020019af 100644 --- a/test/es-module/test-esm-import-attributes-2.mjs +++ b/test/es-module/test-esm-import-attributes-2.mjs @@ -1,9 +1,9 @@ import '../common/index.mjs'; import { strictEqual } from 'assert'; -import secret0 from '../fixtures/experimental.json' assert { type: 'json' }; +import secret0 from '../fixtures/experimental.json' with { type: 'json' }; const secret1 = await import('../fixtures/experimental.json', { - assert: { type: 'json' }, + with: { type: 'json' }, }); strictEqual(secret0.ofLife, 42); diff --git a/test/es-module/test-esm-import-attributes-3.mjs b/test/es-module/test-esm-import-attributes-3.mjs index b9de9232cfff..8950ca5c595e 100644 --- a/test/es-module/test-esm-import-attributes-3.mjs +++ b/test/es-module/test-esm-import-attributes-3.mjs @@ -1,9 +1,9 @@ import '../common/index.mjs'; import { strictEqual } from 'assert'; -import secret0 from '../fixtures/experimental.json' assert { type: 'json' }; +import secret0 from '../fixtures/experimental.json' with { type: 'json' }; const secret1 = await import('../fixtures/experimental.json', - { assert: { type: 'json' } }); + { with: { type: 'json' } }); strictEqual(secret0.ofLife, 42); strictEqual(secret1.default.ofLife, 42); diff --git a/test/es-module/test-esm-import-attributes-errors.js b/test/es-module/test-esm-import-attributes-errors.js index f6e57f19b6b4..4521248db176 100644 --- a/test/es-module/test-esm-import-attributes-errors.js +++ b/test/es-module/test-esm-import-attributes-errors.js @@ -7,27 +7,27 @@ const jsonModuleDataUrl = 'data:application/json,""'; async function test() { await rejects( - import('data:text/css,', { assert: { type: 'css' } }), + import('data:text/css,', { with: { type: 'css' } }), { code: 'ERR_UNKNOWN_MODULE_FORMAT' } ); await rejects( - import('data:text/css,', { assert: { unsupportedAttribute: 'value' } }), + import('data:text/css,', { with: { unsupportedAttribute: 'value' } }), { code: 'ERR_IMPORT_ATTRIBUTE_UNSUPPORTED' } ); await rejects( - import(`data:text/javascript,import${JSON.stringify(jsModuleDataUrl)}assert{type:"json"}`), + import(`data:text/javascript,import${JSON.stringify(jsModuleDataUrl)}with{type:"json"}`), { code: 'ERR_IMPORT_ASSERTION_TYPE_FAILED' } ); await rejects( - import(jsModuleDataUrl, { assert: { type: 'json' } }), + import(jsModuleDataUrl, { with: { type: 'json' } }), { code: 'ERR_IMPORT_ASSERTION_TYPE_FAILED' } ); await rejects( - import(jsModuleDataUrl, { assert: { type: 'unsupported' } }), + import(jsModuleDataUrl, { with: { type: 'unsupported' } }), { code: 'ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED' } ); @@ -37,17 +37,17 @@ async function test() { ); await rejects( - import(jsonModuleDataUrl, { assert: {} }), + import(jsonModuleDataUrl, { with: {} }), { code: 'ERR_IMPORT_ASSERTION_TYPE_MISSING' } ); await rejects( - import(jsonModuleDataUrl, { assert: { foo: 'bar' } }), + import(jsonModuleDataUrl, { with: { foo: 'bar' } }), { code: 'ERR_IMPORT_ASSERTION_TYPE_MISSING' } ); await rejects( - import(jsonModuleDataUrl, { assert: { type: 'unsupported' } }), + import(jsonModuleDataUrl, { with: { type: 'unsupported' } }), { code: 'ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED' } ); } diff --git a/test/es-module/test-esm-import-attributes-errors.mjs b/test/es-module/test-esm-import-attributes-errors.mjs index 6621585be412..ff932636e39a 100644 --- a/test/es-module/test-esm-import-attributes-errors.mjs +++ b/test/es-module/test-esm-import-attributes-errors.mjs @@ -7,22 +7,22 @@ const jsonModuleDataUrl = 'data:application/json,""'; await rejects( // This rejects because of the unsupported MIME type, not because of the // unsupported assertion. - import('data:text/css,', { assert: { type: 'css' } }), + import('data:text/css,', { with: { type: 'css' } }), { code: 'ERR_UNKNOWN_MODULE_FORMAT' } ); await rejects( - import(`data:text/javascript,import${JSON.stringify(jsModuleDataUrl)}assert{type:"json"}`), + import(`data:text/javascript,import${JSON.stringify(jsModuleDataUrl)}with{type:"json"}`), { code: 'ERR_IMPORT_ASSERTION_TYPE_FAILED' } ); await rejects( - import(jsModuleDataUrl, { assert: { type: 'json' } }), + import(jsModuleDataUrl, { with: { type: 'json' } }), { code: 'ERR_IMPORT_ASSERTION_TYPE_FAILED' } ); await rejects( - import(import.meta.url, { assert: { type: 'unsupported' } }), + import(import.meta.url, { with: { type: 'unsupported' } }), { code: 'ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED' } ); @@ -32,16 +32,16 @@ await rejects( ); await rejects( - import(jsonModuleDataUrl, { assert: {} }), + import(jsonModuleDataUrl, { with: {} }), { code: 'ERR_IMPORT_ASSERTION_TYPE_MISSING' } ); await rejects( - import(jsonModuleDataUrl, { assert: { foo: 'bar' } }), + import(jsonModuleDataUrl, { with: { foo: 'bar' } }), { code: 'ERR_IMPORT_ASSERTION_TYPE_MISSING' } ); await rejects( - import(jsonModuleDataUrl, { assert: { type: 'unsupported' } }), + import(jsonModuleDataUrl, { with: { type: 'unsupported' } }), { code: 'ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED' } ); diff --git a/test/es-module/test-esm-json-cache.mjs b/test/es-module/test-esm-json-cache.mjs index b766519d663f..f8d24fd394a4 100644 --- a/test/es-module/test-esm-json-cache.mjs +++ b/test/es-module/test-esm-json-cache.mjs @@ -6,7 +6,7 @@ import { createRequire } from 'module'; import mod from '../fixtures/es-modules/json-cache/mod.cjs'; import another from '../fixtures/es-modules/json-cache/another.cjs'; -import test from '../fixtures/es-modules/json-cache/test.json' assert +import test from '../fixtures/es-modules/json-cache/test.json' with { type: 'json' }; const require = createRequire(import.meta.url); diff --git a/test/es-module/test-esm-json.mjs b/test/es-module/test-esm-json.mjs index e5a0ab9f74e2..422a8f717594 100644 --- a/test/es-module/test-esm-json.mjs +++ b/test/es-module/test-esm-json.mjs @@ -7,7 +7,7 @@ import { describe, it, test } from 'node:test'; import { mkdir, rm, writeFile } from 'node:fs/promises'; import * as tmpdir from '../common/tmpdir.js'; -import secret from '../fixtures/experimental.json' assert { type: 'json' }; +import secret from '../fixtures/experimental.json' with { type: 'json' }; describe('ESM: importing JSON', () => { it('should load JSON', () => { @@ -34,19 +34,19 @@ describe('ESM: importing JSON', () => { const url = new URL('./foo.json', root); await writeFile(url, JSON.stringify({ id: i++ })); const absoluteURL = await import(`${url}`, { - assert: { type: 'json' }, + with: { type: 'json' }, }); await writeFile(url, JSON.stringify({ id: i++ })); const queryString = await import(`${url}?a=2`, { - assert: { type: 'json' }, + with: { type: 'json' }, }); await writeFile(url, JSON.stringify({ id: i++ })); const hash = await import(`${url}#a=2`, { - assert: { type: 'json' }, + with: { type: 'json' }, }); await writeFile(url, JSON.stringify({ id: i++ })); const queryStringAndHash = await import(`${url}?a=2#a=2`, { - assert: { type: 'json' }, + with: { type: 'json' }, }); assert.notDeepStrictEqual(absoluteURL, queryString); diff --git a/test/es-module/test-esm-virtual-json.mjs b/test/es-module/test-esm-virtual-json.mjs index 8876eea013ce..a42b037fc1f2 100644 --- a/test/es-module/test-esm-virtual-json.mjs +++ b/test/es-module/test-esm-virtual-json.mjs @@ -25,6 +25,6 @@ function load(url, context, next) { register(`data:text/javascript,export ${encodeURIComponent(resolve)};export ${encodeURIComponent(load)}`); assert.notDeepStrictEqual( - await import(fixtures.fileURL('empty.json'), { assert: { type: 'json' } }), - await import(fixtures.fileURL('empty.json'), { assert: { type: 'json' } }), + await import(fixtures.fileURL('empty.json'), { with: { type: 'json' } }), + await import(fixtures.fileURL('empty.json'), { with: { type: 'json' } }), ); diff --git a/test/fixtures/es-modules/import-json-named-export.mjs b/test/fixtures/es-modules/import-json-named-export.mjs index 01798c59ac58..be1a4116eb5f 100644 --- a/test/fixtures/es-modules/import-json-named-export.mjs +++ b/test/fixtures/es-modules/import-json-named-export.mjs @@ -1 +1 @@ -import { ofLife } from '../experimental.json' assert { type: 'json' }; +import { ofLife } from '../experimental.json' with { type: 'json' }; diff --git a/test/fixtures/es-modules/json-modules.mjs b/test/fixtures/es-modules/json-modules.mjs index 607c09e51cda..c1eae2b689a6 100644 --- a/test/fixtures/es-modules/json-modules.mjs +++ b/test/fixtures/es-modules/json-modules.mjs @@ -1 +1 @@ -import secret from '../experimental.json' assert { type: 'json' }; +import secret from '../experimental.json' with { type: 'json' }; diff --git a/test/parallel/test-vm-module-dynamic-import.js b/test/parallel/test-vm-module-dynamic-import.js index 5bca08b8c9c3..b74d3b28d7a5 100644 --- a/test/parallel/test-vm-module-dynamic-import.js +++ b/test/parallel/test-vm-module-dynamic-import.js @@ -58,7 +58,7 @@ async function test() { } { - const s = new Script('import("foo", { assert: { key: "value" } })', { + const s = new Script('import("foo", { with: { key: "value" } })', { importModuleDynamically: common.mustCall((specifier, wrap, attributes) => { assert.strictEqual(specifier, 'foo'); assert.strictEqual(wrap, s); From 9cb5138f80430ea46711b72a79efadda240bc213 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Tue, 12 Dec 2023 19:26:04 +0100 Subject: [PATCH 4/5] vm: use import attributes instead of import assertions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The old import assertions proposal has been renamed to "import attributes" with the following major changes: 1. The keyword is now `with` instead of `assert`. 2. Unknown assertions cause an error rather than being ignored. This PR updates the documentation to encourage folks to use the new syntax, and add aliases to preserve backward compatibility. PR-URL: nodejs#50141 Reviewed-By: Geoffrey Booth Reviewed-By: Vinícius Lourenço Claro Cardoso --- doc/api/vm.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/doc/api/vm.md b/doc/api/vm.md index f405d71ef299..cda624731666 100644 --- a/doc/api/vm.md +++ b/doc/api/vm.md @@ -101,7 +101,7 @@ changes: using it in a production environment. * `specifier` {string} specifier passed to `import()` * `script` {vm.Script} - * `importAttributes` {Object} The `"assert"` value passed to the + * `importAttributes` {Object} The `"with"` value passed to the [`optionsExpression`][] optional parameter, or an empty object if no value was provided. * Returns: {Module Namespace Object|vm.Module} Returning a `vm.Module` is @@ -633,7 +633,7 @@ changes: * `extra` {Object} * `attributes` {Object} The data from the attribute: ```mjs - import foo from 'foo' assert { name: 'value' }; + import foo from 'foo' with { name: 'value' }; // ^^^^^^^^^^^^^^^^^ the attribute ``` Per ECMA-262, hosts are expected to trigger an error if an @@ -1025,7 +1025,7 @@ changes: considered stable. * `specifier` {string} specifier passed to `import()` * `function` {Function} - * `importAttributes` {Object} The `"assert"` value passed to the + * `importAttributes` {Object} The `"with"` value passed to the [`optionsExpression`][] optional parameter, or an empty object if no value was provided. * Returns: {Module Namespace Object|vm.Module} Returning a `vm.Module` is @@ -1249,7 +1249,7 @@ changes: using it in a production environment. * `specifier` {string} specifier passed to `import()` * `script` {vm.Script} - * `importAttributes` {Object} The `"assert"` value passed to the + * `importAttributes` {Object} The `"with"` value passed to the [`optionsExpression`][] optional parameter, or an empty object if no value was provided. * Returns: {Module Namespace Object|vm.Module} Returning a `vm.Module` is @@ -1348,7 +1348,7 @@ changes: using it in a production environment. * `specifier` {string} specifier passed to `import()` * `script` {vm.Script} - * `importAttributes` {Object} The `"assert"` value passed to the + * `importAttributes` {Object} The `"with"` value passed to the [`optionsExpression`][] optional parameter, or an empty object if no value was provided. * Returns: {Module Namespace Object|vm.Module} Returning a `vm.Module` is @@ -1428,7 +1428,7 @@ changes: using it in a production environment. * `specifier` {string} specifier passed to `import()` * `script` {vm.Script} - * `importAttributes` {Object} The `"assert"` value passed to the + * `importAttributes` {Object} The `"with"` value passed to the [`optionsExpression`][] optional parameter, or an empty object if no value was provided. * Returns: {Module Namespace Object|vm.Module} Returning a `vm.Module` is From cc239feb03832808b26cd1568b66dede03b09303 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= Date: Sun, 19 Nov 2023 22:22:49 +0100 Subject: [PATCH 5/5] src: iterate on import attributes array correctly The array's length is supposed to be a multiple of two for dynamic import callbacks. Fixes: https://github.com/nodejs/node/issues/50700 PR-URL: https://github.com/nodejs/node/pull/50703 Reviewed-By: Antoine du Hamel Reviewed-By: Shelley Vohr Reviewed-By: Joyee Cheung Reviewed-By: James M Snell --- src/module_wrap.cc | 12 ++++++++---- test/es-module/test-esm-import-attributes-errors.js | 5 +++++ test/es-module/test-esm-import-attributes-errors.mjs | 5 +++++ 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 21b9a702aeba..a11be73b7d4b 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -251,10 +251,14 @@ void ModuleWrap::New(const FunctionCallbackInfo& args) { } static Local createImportAttributesContainer( - Environment* env, Isolate* isolate, Local raw_attributes) { + Environment* env, + Isolate* isolate, + Local raw_attributes, + const int elements_per_attribute) { + CHECK_EQ(raw_attributes->Length() % elements_per_attribute, 0); Local attributes = Object::New(isolate, v8::Null(env->isolate()), nullptr, nullptr, 0); - for (int i = 0; i < raw_attributes->Length(); i += 3) { + for (int i = 0; i < raw_attributes->Length(); i += elements_per_attribute) { attributes ->Set(env->context(), raw_attributes->Get(env->context(), i).As(), @@ -300,7 +304,7 @@ void ModuleWrap::Link(const FunctionCallbackInfo& args) { Local raw_attributes = module_request->GetImportAssertions(); Local attributes = - createImportAttributesContainer(env, isolate, raw_attributes); + createImportAttributesContainer(env, isolate, raw_attributes, 3); Local argv[] = { specifier, @@ -603,7 +607,7 @@ static MaybeLocal ImportModuleDynamically( } Local attributes = - createImportAttributesContainer(env, isolate, import_attributes); + createImportAttributesContainer(env, isolate, import_attributes, 2); Local import_args[] = { object, diff --git a/test/es-module/test-esm-import-attributes-errors.js b/test/es-module/test-esm-import-attributes-errors.js index 4521248db176..976d4a3f67f7 100644 --- a/test/es-module/test-esm-import-attributes-errors.js +++ b/test/es-module/test-esm-import-attributes-errors.js @@ -26,6 +26,11 @@ async function test() { { code: 'ERR_IMPORT_ASSERTION_TYPE_FAILED' } ); + await rejects( + import(jsModuleDataUrl, { with: { type: 'json', other: 'unsupported' } }), + { code: 'ERR_IMPORT_ASSERTION_TYPE_FAILED' } + ); + await rejects( import(jsModuleDataUrl, { with: { type: 'unsupported' } }), { code: 'ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED' } diff --git a/test/es-module/test-esm-import-attributes-errors.mjs b/test/es-module/test-esm-import-attributes-errors.mjs index ff932636e39a..072b94b1b0fc 100644 --- a/test/es-module/test-esm-import-attributes-errors.mjs +++ b/test/es-module/test-esm-import-attributes-errors.mjs @@ -21,6 +21,11 @@ await rejects( { code: 'ERR_IMPORT_ASSERTION_TYPE_FAILED' } ); +await rejects( + import(jsModuleDataUrl, { with: { type: 'json', other: 'unsupported' } }), + { code: 'ERR_IMPORT_ASSERTION_TYPE_FAILED' } +); + await rejects( import(import.meta.url, { with: { type: 'unsupported' } }), { code: 'ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED' }