fix #3792: import attributes and the copy loader

Author: evanw

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`.

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:

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{

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 ----------