From 07f481cfcf1153336534dc5d3cd4c5c9770a72be Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nicol=C3=B2=20Ribaudo?= <hello@nicr.dev>
Date: Fri, 19 Apr 2024 02:01:24 +0200
Subject: [PATCH] esm: drop support for import assertions

This patch removes support for the `assert` keyword
for import attributes. It was an old variant of the
proposal that was only shipped in V8 and no other
engine, and that has then been replaced by the `with`
keyword.

Chrome is planning to remove support for `assert`
in version 126, which will be released in June.

Node.js already supports the `with` keyword for
import attributes, and this patch does not change that.

PR-URL: https://github.com/nodejs/node/pull/52104
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
---
 src/node.cc                                          |  9 +++------
 test/es-module/test-esm-import-attributes-errors.js  | 10 ++++++++++
 test/es-module/test-esm-import-attributes-errors.mjs | 10 ++++++++++
 3 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/src/node.cc b/src/node.cc
index 2395f28c7b8cb7..5d730985bfea4a 100644
--- a/src/node.cc
+++ b/src/node.cc
@@ -800,12 +800,6 @@ static ExitCode ProcessGlobalArgsInternal(std::vector<std::string>* args,
     return ExitCode::kInvalidCommandLineArgument2;
   }
 
-  // TODO(aduh95): remove this when the harmony-import-assertions flag
-  // is removed in V8.
-  if (std::find(v8_args.begin(), v8_args.end(),
-                "--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(),
@@ -813,6 +807,9 @@ static ExitCode ProcessGlobalArgsInternal(std::vector<std::string>* args,
                 "--no-harmony-import-attributes") == v8_args.end()) {
     v8_args.emplace_back("--harmony-import-attributes");
   }
+  // TODO(nicolo-ribaudo): remove this once V8 doesn't enable it by default
+  // anymore.
+  v8_args.emplace_back("--no-harmony-import-assertions");
 
   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-import-attributes-errors.js b/test/es-module/test-esm-import-attributes-errors.js
index 828ded7e684342..63513a51e5abb5 100644
--- a/test/es-module/test-esm-import-attributes-errors.js
+++ b/test/es-module/test-esm-import-attributes-errors.js
@@ -55,6 +55,16 @@ async function test() {
     import(jsonModuleDataUrl, { with: { type: 'unsupported' } }),
     { code: 'ERR_IMPORT_ATTRIBUTE_UNSUPPORTED' }
   );
+
+  await rejects(
+    import(jsonModuleDataUrl, { assert: { type: 'json' } }),
+    { code: 'ERR_IMPORT_ATTRIBUTE_MISSING' }
+  );
+
+  await rejects(
+    import(`data:text/javascript,import${JSON.stringify(jsonModuleDataUrl)}assert{type:"json"}`),
+    SyntaxError
+  );
 }
 
 test().then(common.mustCall());
diff --git a/test/es-module/test-esm-import-attributes-errors.mjs b/test/es-module/test-esm-import-attributes-errors.mjs
index 887a1b97cc26ee..d7ffc6f92c99ea 100644
--- a/test/es-module/test-esm-import-attributes-errors.mjs
+++ b/test/es-module/test-esm-import-attributes-errors.mjs
@@ -50,3 +50,13 @@ await rejects(
   import(jsonModuleDataUrl, { with: { type: 'unsupported' } }),
   { code: 'ERR_IMPORT_ATTRIBUTE_UNSUPPORTED' }
 );
+
+await rejects(
+  import(jsonModuleDataUrl, { assert: { type: 'json' } }),
+  { code: 'ERR_IMPORT_ATTRIBUTE_MISSING' }
+);
+
+await rejects(
+  import(`data:text/javascript,import${JSON.stringify(jsonModuleDataUrl)}assert{type:"json"}`),
+  SyntaxError
+);