fix #3778: import assertions/attributes for node

evanw · evanw · commit 5069410dafd9 · 2024-05-24T12:33:48.000-04:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,23 @@
 
 ## Unreleased
 
+* Update support for import assertions and import attributes in node ([#3778](https://github.com/evanw/esbuild/issues/3778))
+
+    Import assertions (the `assert` keyword) have been removed from node starting in v22.0.0. So esbuild will now strip them and generate a warning with `--target=node22` or above:
+
+    ```
+    ▲ [WARNING] The "assert" keyword is not supported in the configured target environment ("node22") [assert-to-with]
+
+        example.mjs:1:40:
+          1 │ import json from "esbuild/package.json" assert { type: "json" }
+            │                                         ~~~~~~
+            ╵                                         with
+
+      Did you mean to use "with" instead of "assert"?
+    ```
+
+    Import attributes (the `with` keyword) have been backported to node 18 starting in v18.20.0. So esbuild will no longer strip them with `--target=node18.N` if `N` is 20 or greater.
+
 * Fix `for await` transform when a label is present
 
     This release fixes a bug where the `for await` transform, which wraps the loop in a `try` statement, previously failed to also move the loop's label into the `try` statement. This bug only affects code that uses both of these features in combination. Here's an example of some affected code:
diff --git a/compat-table/src/index.ts b/compat-table/src/index.ts
@@ -477,14 +477,29 @@ import('./kangax').then(kangax => {
 
   // Import assertions (note: these were removed from the JavaScript specification and never standardized)
   {
-    // From https://github.com/nodejs/node/blob/master/doc/changelogs/CHANGELOG_V16.md#16.14.0
-    js.ImportAssertions.Node = { '16.14': { force: true } }
+    js.ImportAssertions.Node = {
+      // From https://github.com/nodejs/node/blob/master/doc/changelogs/CHANGELOG_V16.md#16.14.0
+      '16.14': { force: true },
+
+      // Manually tested using a binary from https://nodejs.org/en/download/prebuilt-binaries
+      '22': { force: false },
+    }
 
     // MDN data is wrong here: https://bugs.webkit.org/show_bug.cgi?id=251600
     delete js.ImportAssertions.IOS
     delete js.ImportAssertions.Safari
   }
 
+  // Import attributes (the replacement for import assertions)
+  {
+    // Manually tested using binaries from https://nodejs.org/en/download/prebuilt-binaries
+    js.ImportAttributes.Node = {
+      '18.20': { force: true },
+      '19': { force: false },
+      '20.10': { force: true },
+    }
+  }
+
   // MDN data is wrong here: https://www.chromestatus.com/feature/6482797915013120
   js.ClassStaticBlocks.Chrome = { 91: { force: true } }
 
diff --git a/internal/compat/js_table.go b/internal/compat/js_table.go
@@ -584,14 +584,14 @@ var jsTable = map[JSFeature]map[Engine][]versionRange{
 		Chrome: {{start: v{91, 0, 0}}},
 		Deno:   {{start: v{1, 17, 0}}},
 		Edge:   {{start: v{91, 0, 0}}},
-		Node:   {{start: v{16, 14, 0}}},
+		Node:   {{start: v{16, 14, 0}, end: v{22, 0, 0}}},
 	},
 	ImportAttributes: {
 		Chrome: {{start: v{123, 0, 0}}},
 		Deno:   {{start: v{1, 37, 0}}},
 		Edge:   {{start: v{123, 0, 0}}},
 		IOS:    {{start: v{17, 2, 0}}},
-		Node:   {{start: v{20, 10, 0}}},
+		Node:   {{start: v{18, 20, 0}, end: v{19, 0, 0}}, {start: v{20, 10, 0}}},
 		Opera:  {{start: v{109, 0, 0}}},
 		Safari: {{start: v{17, 2, 0}}},
 	},
diff --git a/internal/js_parser/js_parser.go b/internal/js_parser/js_parser.go
@@ -6480,6 +6480,9 @@ func (p *parser) parsePath() (logger.Range, string, *ast.ImportAssertOrWith, ast
 
 		closeBraceLoc := p.saveExprCommentsHere()
 		p.lexer.Expect(js_lexer.TCloseBrace)
+		if keyword == ast.AssertKeyword {
+			p.maybeWarnAboutAssertKeyword(keywordLoc)
+		}
 		assertOrWith = &ast.ImportAssertOrWith{
 			Entries:            entries,
 			Keyword:            keyword,
@@ -6492,6 +6495,20 @@ func (p *parser) parsePath() (logger.Range, string, *ast.ImportAssertOrWith, ast
 	return pathRange, pathText, assertOrWith, flags
 }
 
+// Let people know if they probably should be using "with" instead of "assert"
+func (p *parser) maybeWarnAboutAssertKeyword(loc logger.Loc) {
+	if p.options.unsupportedJSFeatures.Has(compat.ImportAssertions) && !p.options.unsupportedJSFeatures.Has(compat.ImportAttributes) {
+		where := config.PrettyPrintTargetEnvironment(p.options.originalTargetEnv, p.options.unsupportedJSFeatureOverridesMask)
+		msg := logger.Msg{
+			Kind:  logger.Warning,
+			Data:  p.tracker.MsgData(js_lexer.RangeOfIdentifier(p.source, loc), "The \"assert\" keyword is not supported in "+where),
+			Notes: []logger.MsgData{{Text: "Did you mean to use \"with\" instead of \"assert\"?"}},
+		}
+		msg.Data.Location.Suggestion = "with"
+		p.log.AddMsgID(logger.MsgID_JS_AssertToWith, msg)
+	}
+}
+
 // This assumes the "function" token has already been parsed
 func (p *parser) parseFnStmt(loc logger.Loc, opts parseStmtOpts, isAsync bool, asyncRange logger.Range) js_ast.Stmt {
 	isGenerator := p.lexer.Token == js_lexer.TAsterisk
@@ -14277,6 +14294,9 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
 									break
 								}
 								if entries != nil {
+									if keyword == ast.AssertKeyword {
+										p.maybeWarnAboutAssertKeyword(prop.Key.Loc)
+									}
 									assertOrWith = &ast.ImportAssertOrWith{
 										Entries:            entries,
 										Keyword:            keyword,
diff --git a/internal/js_parser/js_parser_test.go b/internal/js_parser/js_parser_test.go
@@ -6229,6 +6229,17 @@ func TestImportAttributes(t *testing.T) {
 	expectPrintedWithUnsupportedFeatures(t, compat.ImportAssertions|compat.ImportAttributes,
 		"import 'x' with {y: 'z'}; import('x', {with: {y: 'z'}})",
 		"import \"x\";\nimport(\"x\");\n")
+
+	// Test the migration warning
+	expectParseErrorWithUnsupportedFeatures(t, compat.ImportAssertions,
+		"import x from 'y' assert {type: 'json'}",
+		"<stdin>: WARNING: The \"assert\" keyword is not supported in the configured target environment\nNOTE: Did you mean to use \"with\" instead of \"assert\"?\n")
+	expectParseErrorWithUnsupportedFeatures(t, compat.ImportAssertions,
+		"export {default} from 'y' assert {type: 'json'}",
+		"<stdin>: WARNING: The \"assert\" keyword is not supported in the configured target environment\nNOTE: Did you mean to use \"with\" instead of \"assert\"?\n")
+	expectParseErrorWithUnsupportedFeatures(t, compat.ImportAssertions,
+		"import('y', {assert: {type: 'json'}})",
+		"<stdin>: WARNING: The \"assert\" keyword is not supported in the configured target environment\nNOTE: Did you mean to use \"with\" instead of \"assert\"?\n")
 }
 
 func TestES5(t *testing.T) {
diff --git a/internal/logger/msg_ids.go b/internal/logger/msg_ids.go
@@ -12,6 +12,7 @@ const (
 	MsgID_None MsgID = iota
 
 	// JavaScript
+	MsgID_JS_AssertToWith
 	MsgID_JS_AssertTypeJSON
 	MsgID_JS_AssignToConstant
 	MsgID_JS_AssignToDefine
@@ -96,6 +97,8 @@ const (
 func StringToMsgIDs(str string, logLevel LogLevel, overrides map[MsgID]LogLevel) {
 	switch str {
 	// JS
+	case "assert-to-with":
+		overrides[MsgID_JS_AssertToWith] = logLevel
 	case "assert-type-json":
 		overrides[MsgID_JS_AssertTypeJSON] = logLevel
 	case "assign-to-constant":
@@ -226,6 +229,8 @@ func StringToMsgIDs(str string, logLevel LogLevel, overrides map[MsgID]LogLevel)
 func MsgIDToString(id MsgID) string {
 	switch id {
 	// JS
+	case MsgID_JS_AssertToWith:
+		return "assert-to-with"
 	case MsgID_JS_AssertTypeJSON:
 		return "assert-type-json"
 	case MsgID_JS_AssignToConstant:
