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

[v18.x backport] esm: use import attributes instead of import assertions #51136

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 1 addition & 4 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
7 changes: 3 additions & 4 deletions deps/v8/include/v8-script.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<FixedArray> GetImportAssertions() const;

Expand Down
47 changes: 35 additions & 12 deletions deps/v8/src/execution/isolate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4726,7 +4726,7 @@ MaybeHandle<FixedArray> 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(
Expand All @@ -4736,18 +4736,35 @@ MaybeHandle<FixedArray> Isolate::GetImportAssertionsFromArgument(

Handle<JSReceiver> import_assertions_argument_receiver =
Handle<JSReceiver>::cast(import_assertions_argument);
Handle<Name> key = factory()->assert_string();

Handle<Object> 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<FixedArray>();

if (FLAG_harmony_import_attributes) {
Handle<Name> 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<FixedArray>();
}
}

// 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<Name> 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<FixedArray>();
}
}

// 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()) {
Expand All @@ -4769,6 +4786,8 @@ MaybeHandle<FixedArray> Isolate::GetImportAssertionsFromArgument(
return MaybeHandle<FixedArray>();
}

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;
Expand All @@ -4786,9 +4805,7 @@ MaybeHandle<FixedArray> Isolate::GetImportAssertionsFromArgument(
}

if (!assertion_value->IsString()) {
this->Throw(*factory()->NewTypeError(
MessageTemplate::kNonStringImportAssertionValue));
return MaybeHandle<FixedArray>();
has_non_string_attribute = true;
}

import_assertions_array->set((i * kAssertionEntrySizeForDynamicImport),
Expand All @@ -4797,6 +4814,12 @@ MaybeHandle<FixedArray> Isolate::GetImportAssertionsFromArgument(
*assertion_value);
}

if (has_non_string_attribute) {
this->Throw(*factory()->NewTypeError(
MessageTemplate::kNonStringImportAssertionValue));
return MaybeHandle<FixedArray>();
}

return import_assertions_array;
}

Expand Down
1 change: 1 addition & 0 deletions deps/v8/src/flags/flag-definitions.h
Original file line number Diff line number Diff line change
Expand Up @@ -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") \
Expand Down
1 change: 1 addition & 0 deletions deps/v8/src/init/bootstrapper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4482,6 +4482,7 @@ void Genesis::InitializeConsole(Handle<JSObject> 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)
Expand Down
1 change: 1 addition & 0 deletions deps/v8/src/init/heap-symbols.h
Original file line number Diff line number Diff line change
Expand Up @@ -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") \
Expand Down
4 changes: 3 additions & 1 deletion deps/v8/src/parsing/parser-base.h
Original file line number Diff line number Diff line change
Expand Up @@ -3725,7 +3725,9 @@ ParserBase<Impl>::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);
Expand Down
45 changes: 24 additions & 21 deletions deps/v8/src/parsing/parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1344,39 +1344,42 @@ ZonePtrList<const Parser::NamedImport>* 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<ImportAssertions>(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;
}

Expect(Token::LBRACE);

while (peek() != Token::RBRACE) {
const AstRawString* attribute_key = nullptr;
if (Check(Token::STRING)) {
attribute_key = GetSymbol();
} else {
attribute_key = ParsePropertyName();
}
const AstRawString* attribute_key =
Check(Token::STRING) ? GetSymbol() : ParsePropertyName();

Scanner::Location location = scanner()->location();

Expand Down
9 changes: 9 additions & 0 deletions deps/v8/test/mjsunit/harmony/modules-import-attributes-1.mjs
Original file line number Diff line number Diff line change
@@ -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());
9 changes: 9 additions & 0 deletions deps/v8/test/mjsunit/harmony/modules-import-attributes-2.mjs
Original file line number Diff line number Diff line change
@@ -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);
9 changes: 9 additions & 0 deletions deps/v8/test/mjsunit/harmony/modules-import-attributes-3.mjs
Original file line number Diff line number Diff line change
@@ -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());
9 changes: 9 additions & 0 deletions deps/v8/test/mjsunit/harmony/modules-import-attributes-4.mjs
Original file line number Diff line number Diff line change
@@ -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);
Original file line number Diff line number Diff line change
@@ -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);
Original file line number Diff line number Diff line change
@@ -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);
Original file line number Diff line number Diff line change
@@ -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);
Original file line number Diff line number Diff line change
@@ -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);
Original file line number Diff line number Diff line change
@@ -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);
Original file line number Diff line number Diff line change
@@ -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);
Original file line number Diff line number Diff line change
@@ -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);