diff --git a/CHANGELOG.md b/CHANGELOG.md index c9131748129..9a980049096 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,24 @@ ## Unreleased +* Allow unknown import attributes to be used with the `copy` loader ([#3792](https://github.com/evanw/esbuild/issues/3792)) + + Import attributes (the `with` keyword on `import` statements) are allowed to alter how that path is loaded. For example, esbuild cannot assume that it knows how to load `./bagel.js` as type `bagel`: + + ```js + // This is an error with "--bundle" without also using "--external:./bagel.js" + import tasty from "./bagel.js" with { type: "bagel" } + ``` + + Because of that, bundling this code with esbuild is an error unless the file `./bagel.js` is external to the bundle (such as with `--bundle --external:./bagel.js`). + + However, there is an additional case where it's ok for esbuild to allow this: if the file is loaded using the `copy` loader. That's because the `copy` loader behaves similarly to `--external` in that the file is left external to the bundle. The difference is that the `copy` loader copies the file into the output folder and rewrites the import path while `--external` doesn't. That means the following will now work with the `copy` loader (such as with `--bundle --loader:.bagel=copy`): + + ```js + // This is no longer an error with "--bundle" and "--loader:.bagel=copy" + import tasty from "./tasty.bagel" with { type: "bagel" } + ``` + * Fix internal error with `--supported:object-accessors=false` ([#3794](https://github.com/evanw/esbuild/issues/3794)) This release fixes a regression in 0.21.0 where some code that was added to esbuild's internal runtime library of helper functions for JavaScript decorators fails to parse when you configure esbuild with `--supported:object-accessors=false`. The reason is that esbuild introduced code that does `{ get [name]() {} }` which uses both the `object-extensions` feature for the `[name]` and the `object-accessors` feature for the `get`, but esbuild was incorrectly only checking for `object-extensions` and not for `object-accessors`. Additional tests have been added to avoid this type of issue in the future. A workaround for this issue in earlier releases is to also add `--supported:object-extensions=false`. diff --git a/internal/bundler/bundler.go b/internal/bundler/bundler.go index dd99f7d546f..31684b85960 100644 --- a/internal/bundler/bundler.go +++ b/internal/bundler/bundler.go @@ -149,7 +149,6 @@ func parseFile(args parseArgs) { &source, args.importSource, args.importPathRange, - args.importWith, args.pluginData, args.options.WatchMode, ) @@ -175,6 +174,44 @@ func parseFile(args parseArgs) { loader = loaderFromFileExtension(args.options.ExtensionToLoader, base+ext) } + // Reject unsupported import attributes when the loader isn't "copy" (since + // "copy" is kind of like "external"). But only do this if this file was not + // loaded by a plugin. Plugins are allowed to assign whatever semantics they + // want to import attributes. + if loader != config.LoaderCopy && pluginName == "" { + for _, attr := range source.KeyPath.ImportAttributes.DecodeIntoArray() { + var errorText string + var errorRange js_lexer.KeyOrValue + + // We only currently handle "type: json" + if attr.Key != "type" { + errorText = fmt.Sprintf("Importing with the %q attribute is not supported", attr.Key) + errorRange = js_lexer.KeyRange + } else if attr.Value == "json" { + loader = config.LoaderWithTypeJSON + continue + } else { + errorText = fmt.Sprintf("Importing with a type attribute of %q is not supported", attr.Value) + errorRange = js_lexer.ValueRange + } + + // Everything else is an error + r := args.importPathRange + if args.importWith != nil { + r = js_lexer.RangeOfImportAssertOrWith(*args.importSource, *ast.FindAssertOrWithEntry(args.importWith.Entries, attr.Key), errorRange) + } + tracker := logger.MakeLineColumnTracker(args.importSource) + args.log.AddError(&tracker, r, errorText) + if args.inject != nil { + args.inject <- config.InjectedFile{ + Source: source, + } + } + args.results <- parseResult{} + return + } + } + if loader == config.LoaderEmpty { source.Contents = "" } @@ -991,7 +1028,6 @@ func runOnLoadPlugins( source *logger.Source, importSource *logger.Source, importPathRange logger.Range, - importWith *ast.ImportAssertOrWith, pluginData interface{}, isWatchMode bool, ) (loaderPluginResult, bool) { @@ -1058,30 +1094,6 @@ func runOnLoadPlugins( } } - // Reject unsupported import attributes - loader := config.LoaderDefault - for _, attr := range source.KeyPath.ImportAttributes.DecodeIntoArray() { - if attr.Key == "type" { - if attr.Value == "json" { - loader = config.LoaderWithTypeJSON - } else { - r := importPathRange - if importWith != nil { - r = js_lexer.RangeOfImportAssertOrWith(*importSource, *ast.FindAssertOrWithEntry(importWith.Entries, attr.Key), js_lexer.ValueRange) - } - log.AddError(&tracker, r, fmt.Sprintf("Importing with a type attribute of %q is not supported", attr.Value)) - return loaderPluginResult{}, false - } - } else { - r := importPathRange - if importWith != nil { - r = js_lexer.RangeOfImportAssertOrWith(*importSource, *ast.FindAssertOrWithEntry(importWith.Entries, attr.Key), js_lexer.KeyRange) - } - log.AddError(&tracker, r, fmt.Sprintf("Importing with the %q attribute is not supported", attr.Key)) - return loaderPluginResult{}, false - } - } - // Force disabled modules to be empty if source.KeyPath.IsDisabled() { return loaderPluginResult{loader: config.LoaderEmpty}, true @@ -1092,7 +1104,7 @@ func runOnLoadPlugins( if contents, err, originalError := fsCache.ReadFile(fs, source.KeyPath.Text); err == nil { source.Contents = contents return loaderPluginResult{ - loader: loader, + loader: config.LoaderDefault, absResolveDir: fs.Dir(source.KeyPath.Text), }, true } else { @@ -1121,9 +1133,6 @@ func runOnLoadPlugins( return loaderPluginResult{loader: config.LoaderNone}, true } else { source.Contents = contents - if loader != config.LoaderDefault { - return loaderPluginResult{loader: loader}, true - } if mimeType := parsed.DecodeMIMEType(); mimeType != resolver.MIMETypeUnsupported { switch mimeType { case resolver.MIMETypeTextCSS: diff --git a/internal/bundler_tests/bundler_loader_test.go b/internal/bundler_tests/bundler_loader_test.go index 9346392a1e9..7ed20b7b0ee 100644 --- a/internal/bundler_tests/bundler_loader_test.go +++ b/internal/bundler_tests/bundler_loader_test.go @@ -1614,6 +1614,55 @@ func TestLoaderBundleWithImportAttributes(t *testing.T) { }) } +func TestLoaderBundleWithUnknownImportAttributesAndJSLoader(t *testing.T) { + loader_suite.expectBundled(t, bundled{ + files: map[string]string{ + "/entry.js": ` + import foo from "./foo.js" with { type: 'js' } + import bar from "./bar.js" with { js: 'true' } + import foo2 from "data:text/javascript,foo" with { type: 'js' } + import bar2 from "data:text/javascript,bar" with { js: 'true' } + console.log(foo, bar, foo2, bar2) + `, + "/foo.js": `...`, + "/bar.js": `,,,`, + }, + entryPaths: []string{"/entry.js"}, + options: config.Options{ + Mode: config.ModeBundle, + AbsOutputFile: "/out.js", + }, + expectedScanLog: `entry.js: ERROR: Importing with a type attribute of "js" is not supported +entry.js: ERROR: Importing with the "js" attribute is not supported +entry.js: ERROR: Importing with a type attribute of "js" is not supported +entry.js: ERROR: Importing with the "js" attribute is not supported +`, + }) +} + +func TestLoaderBundleWithUnknownImportAttributesAndCopyLoader(t *testing.T) { + loader_suite.expectBundled(t, bundled{ + files: map[string]string{ + "/entry.js": ` + import foo from "./foo.thing" with { type: 'whatever' } + import bar from "./bar.thing" with { whatever: 'true' } + console.log(foo, bar) + `, + "/foo.thing": `...`, + "/bar.thing": `,,,`, + }, + entryPaths: []string{"/entry.js"}, + options: config.Options{ + Mode: config.ModeBundle, + ExtensionToLoader: map[string]config.Loader{ + ".js": config.LoaderJS, + ".thing": config.LoaderCopy, + }, + AbsOutputFile: "/out.js", + }, + }) +} + func TestLoaderBundleWithTypeJSONOnlyDefaultExport(t *testing.T) { loader_suite.expectBundled(t, bundled{ files: map[string]string{ diff --git a/internal/bundler_tests/snapshots/snapshots_loader.txt b/internal/bundler_tests/snapshots/snapshots_loader.txt index 8bb4c4bb1d9..59995d13b86 100644 --- a/internal/bundler_tests/snapshots/snapshots_loader.txt +++ b/internal/bundler_tests/snapshots/snapshots_loader.txt @@ -268,6 +268,18 @@ var data_default2 = { works: true }; // entry.js console.log(data_default === data_default, data_default !== data_default2); +================================================================================ +TestLoaderBundleWithUnknownImportAttributesAndCopyLoader +---------- /foo-AKINYSFH.thing ---------- +... +---------- /bar-AXZXSLHF.thing ---------- +,,, +---------- /out.js ---------- +// entry.js +import foo from "./foo-AKINYSFH.thing" with { type: "whatever" }; +import bar from "./bar-AXZXSLHF.thing" with { whatever: "true" }; +console.log(foo, bar); + ================================================================================ TestLoaderCopyEntryPointAdvanced ---------- /out/xyz-DYPYXS7B.copy ----------