From 5069410dafd95f44b7d66732d8bfd0aef6321c3b Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Fri, 24 May 2024 12:33:27 -0400 Subject: [PATCH] fix #3778: import assertions/attributes for node --- CHANGELOG.md | 17 +++++++++++++++++ compat-table/src/index.ts | 19 +++++++++++++++++-- internal/compat/js_table.go | 4 ++-- internal/js_parser/js_parser.go | 20 ++++++++++++++++++++ internal/js_parser/js_parser_test.go | 11 +++++++++++ internal/logger/msg_ids.go | 5 +++++ 6 files changed, 72 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ca7ffc76562..26ad1fb0d76 100644 --- 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 index 2e3804c231d..a98372b62dd 100644 --- 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 index 808eca3c69b..ea3d7f7069b 100644 --- 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 index 574b26e2c1b..3a2e63a1002 100644 --- 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 index 10247fb5404..bf1f2b9c1f7 100644 --- 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'}", + ": 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'}", + ": 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'}})", + ": 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 index 3f2773da08b..bcc9c9b2ce9 100644 --- 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: