Skip to content

Commit

Permalink
fix #3778: import assertions/attributes for node
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed May 24, 2024
1 parent 11d568c commit 5069410
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 4 deletions.
17 changes: 17 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
19 changes: 17 additions & 2 deletions compat-table/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 } }

Expand Down
4 changes: 2 additions & 2 deletions internal/compat/js_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}}},
},
Expand Down
20 changes: 20 additions & 0 deletions internal/js_parser/js_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand Down
11 changes: 11 additions & 0 deletions internal/js_parser/js_parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
5 changes: 5 additions & 0 deletions internal/logger/msg_ids.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const (
MsgID_None MsgID = iota

// JavaScript
MsgID_JS_AssertToWith
MsgID_JS_AssertTypeJSON
MsgID_JS_AssignToConstant
MsgID_JS_AssignToDefine
Expand Down Expand Up @@ -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":
Expand Down Expand Up @@ -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:
Expand Down

0 comments on commit 5069410

Please sign in to comment.