Skip to content

Commit

Permalink
fix #619: make --external: more consistent
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Jan 22, 2022
1 parent 572240c commit 345f64c
Show file tree
Hide file tree
Showing 9 changed files with 191 additions and 169 deletions.
10 changes: 10 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,16 @@

## Unreleased

* Be more consistent about external paths ([#619](https://github.com/evanw/esbuild/issues/619))

The rules for marking paths as external using `--external:` grew over time as more special-cases were added. This release reworks the internal representation to be more straightforward and robust. A side effect is that wildcard patterns can now match post-resolve paths in addition to pre-resolve paths. Specifically you can now do `--external:./node_modules/*` to mark all files in the `./node_modules/` directory as external.

This is the updated logic:

* Before path resolution begins, import paths are checked against everything passed via an `--external:` flag. In addition, if something looks like a package path (i.e. doesn't start with `/` or `./` or `../`), import paths are checked to see if they have that package path as a path prefix (so `--external:@foo/bar` matches the import path `@foo/bar/baz`).

* After path resolution ends, the absolute paths are checked against everything passed via `--external:` that doesn't look like a package path (i.e. that starts with `/` or `./` or `../`). But before checking, the pattern is transformed to be relative to the current working directory.

* Attempt to explain why esbuild can't run ([#1819](https://github.com/evanw/esbuild/issues/1819))

People sometimes try to install esbuild on one OS and then copy the `node_modules` directory over to another OS without reinstalling. This works with JavaScript code but doesn't work with esbuild because esbuild is a native binary executable. This release attempts to offer a helpful error message when this happens. It looks like this:
Expand Down
24 changes: 12 additions & 12 deletions internal/bundler/bundler_css_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,14 +83,14 @@ func TestCSSAtImportExternal(t *testing.T) {
options: config.Options{
Mode: config.ModeBundle,
AbsOutputDir: "/out",
ExternalModules: config.ExternalModules{
AbsPaths: map[string]bool{
ExternalSettings: config.ExternalSettings{
PostResolve: config.ExternalMatchers{Exact: map[string]bool{
"/external1.css": true,
"/external2.css": true,
"/external3.css": true,
"/external4.css": true,
"/external5.css": true,
},
}},
},
},
})
Expand Down Expand Up @@ -296,10 +296,10 @@ func TestExternalImportURLInCSS(t *testing.T) {
options: config.Options{
Mode: config.ModeBundle,
AbsOutputDir: "/out",
ExternalModules: config.ExternalModules{
AbsPaths: map[string]bool{
ExternalSettings: config.ExternalSettings{
PostResolve: config.ExternalMatchers{Exact: map[string]bool{
"/src/external.png": true,
},
}},
},
},
})
Expand Down Expand Up @@ -652,10 +652,10 @@ func TestCSSExternalQueryAndHashNoMatchIssue1822(t *testing.T) {
options: config.Options{
Mode: config.ModeBundle,
AbsOutputFile: "/out.css",
ExternalModules: config.ExternalModules{
Patterns: []config.WildcardPattern{
ExternalSettings: config.ExternalSettings{
PreResolve: config.ExternalMatchers{Patterns: []config.WildcardPattern{
{Suffix: ".png"},
},
}},
},
},
expectedScanLog: `entry.css: ERROR: Could not resolve "foo/bar.png?baz"
Expand All @@ -678,11 +678,11 @@ func TestCSSExternalQueryAndHashMatchIssue1822(t *testing.T) {
options: config.Options{
Mode: config.ModeBundle,
AbsOutputFile: "/out.css",
ExternalModules: config.ExternalModules{
Patterns: []config.WildcardPattern{
ExternalSettings: config.ExternalSettings{
PreResolve: config.ExternalMatchers{Patterns: []config.WildcardPattern{
{Suffix: ".png?baz"},
{Suffix: ".png#baz"},
},
}},
},
},
})
Expand Down
136 changes: 73 additions & 63 deletions internal/bundler/bundler_default_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -912,12 +912,12 @@ func TestConditionalRequireResolve(t *testing.T) {
Platform: config.PlatformNode,
OutputFormat: config.FormatCommonJS,
AbsOutputFile: "/out.js",
ExternalModules: config.ExternalModules{
NodeModules: map[string]bool{
ExternalSettings: config.ExternalSettings{
PreResolve: config.ExternalMatchers{Exact: map[string]bool{
"a": true,
"b": true,
"c": true,
},
}},
},
},
})
Expand All @@ -938,11 +938,11 @@ func TestConditionalRequire(t *testing.T) {
options: config.Options{
Mode: config.ModeBundle,
AbsOutputFile: "/out.js",
ExternalModules: config.ExternalModules{
NodeModules: map[string]bool{
ExternalSettings: config.ExternalSettings{
PreResolve: config.ExternalMatchers{Exact: map[string]bool{
"a": true,
"c": true,
},
}},
},
},
})
Expand All @@ -965,11 +965,11 @@ func TestConditionalImport(t *testing.T) {
options: config.Options{
Mode: config.ModeBundle,
AbsOutputDir: "/out",
ExternalModules: config.ExternalModules{
NodeModules: map[string]bool{
ExternalSettings: config.ExternalSettings{
PreResolve: config.ExternalMatchers{Exact: map[string]bool{
"a": true,
"c": true,
},
}},
},
},
})
Expand Down Expand Up @@ -2093,10 +2093,10 @@ func TestImportReExportES6Issue149(t *testing.T) {
Factory: config.JSXExpr{Parts: []string{"h"}},
},
AbsOutputFile: "/out.js",
ExternalModules: config.ExternalModules{
NodeModules: map[string]bool{
ExternalSettings: config.ExternalSettings{
PreResolve: config.ExternalMatchers{Exact: map[string]bool{
"preact": true,
},
}},
},
},
})
Expand All @@ -2116,10 +2116,12 @@ func TestExternalModuleExclusionPackage(t *testing.T) {
options: config.Options{
Mode: config.ModeBundle,
AbsOutputFile: "/out.js",
ExternalModules: config.ExternalModules{
NodeModules: map[string]bool{
ExternalSettings: config.ExternalSettings{
PreResolve: config.ExternalMatchers{Exact: map[string]bool{
"aws-sdk": true,
},
}, Patterns: []config.WildcardPattern{
{Prefix: "aws-sdk/"},
}},
},
},
})
Expand Down Expand Up @@ -2149,12 +2151,16 @@ func TestExternalModuleExclusionScopedPackage(t *testing.T) {
options: config.Options{
Mode: config.ModeBundle,
AbsOutputFile: "/out.js",
ExternalModules: config.ExternalModules{
NodeModules: map[string]bool{
ExternalSettings: config.ExternalSettings{
PreResolve: config.ExternalMatchers{Exact: map[string]bool{
"@a1": true,
"@b1/b2": true,
"@c1/c2/c3": true,
},
}, Patterns: []config.WildcardPattern{
{Prefix: "@a1/"},
{Prefix: "@b1/b2/"},
{Prefix: "@c1/c2/c3/"},
}},
},
},
expectedScanLog: `index.js: ERROR: Could not resolve "@a1-a2"
Expand Down Expand Up @@ -2187,10 +2193,12 @@ func TestScopedExternalModuleExclusion(t *testing.T) {
options: config.Options{
Mode: config.ModeBundle,
AbsOutputFile: "/out.js",
ExternalModules: config.ExternalModules{
NodeModules: map[string]bool{
ExternalSettings: config.ExternalSettings{
PreResolve: config.ExternalMatchers{Exact: map[string]bool{
"@scope/foo": true,
},
}, Patterns: []config.WildcardPattern{
{Prefix: "@scope/foo/"},
}},
},
},
})
Expand All @@ -2214,13 +2222,15 @@ func TestExternalModuleExclusionRelativePath(t *testing.T) {
options: config.Options{
Mode: config.ModeBundle,
AbsOutputDir: "/Users/user/project/out",
ExternalModules: config.ExternalModules{
AbsPaths: map[string]bool{
ExternalSettings: config.ExternalSettings{
PostResolve: config.ExternalMatchers{Exact: map[string]bool{
"/Users/user/project/out/in-out-dir.js": true,
"/Users/user/project/src/nested/folder/foo.js": true,
"/Users/user/project/src/sha256.min.js": true,
"/api/config?a=1&b=2": true,
},
}},
PreResolve: config.ExternalMatchers{Exact: map[string]bool{
"/api/config?a=1&b=2": true,
}},
},
},
})
Expand Down Expand Up @@ -2405,12 +2415,12 @@ func TestExternalWithWildcard(t *testing.T) {
options: config.Options{
Mode: config.ModeBundle,
AbsOutputDir: "/out",
ExternalModules: config.ExternalModules{
Patterns: []config.WildcardPattern{
ExternalSettings: config.ExternalSettings{
PreResolve: config.ExternalMatchers{Patterns: []config.WildcardPattern{
{Prefix: "/assets/"},
{Suffix: ".png"},
{Prefix: "/dir/", Suffix: "/file.gif"},
},
}},
},
},
expectedScanLog: `entry.js: ERROR: Could not resolve "/sassets/images/test.jpg"
Expand Down Expand Up @@ -2899,11 +2909,11 @@ func TestReExportDefaultExternalES6(t *testing.T) {
Mode: config.ModeBundle,
AbsOutputFile: "/out.js",
OutputFormat: config.FormatESModule,
ExternalModules: config.ExternalModules{
NodeModules: map[string]bool{
ExternalSettings: config.ExternalSettings{
PreResolve: config.ExternalMatchers{Exact: map[string]bool{
"foo": true,
"bar": true,
},
}},
},
},
})
Expand All @@ -2925,11 +2935,11 @@ func TestReExportDefaultExternalCommonJS(t *testing.T) {
Mode: config.ModeBundle,
AbsOutputFile: "/out.js",
OutputFormat: config.FormatCommonJS,
ExternalModules: config.ExternalModules{
NodeModules: map[string]bool{
ExternalSettings: config.ExternalSettings{
PreResolve: config.ExternalMatchers{Exact: map[string]bool{
"foo": true,
"bar": true,
},
}},
},
},
})
Expand Down Expand Up @@ -3814,14 +3824,14 @@ func TestRequireResolve(t *testing.T) {
Platform: config.PlatformNode,
OutputFormat: config.FormatCommonJS,
AbsOutputFile: "/out.js",
ExternalModules: config.ExternalModules{
AbsPaths: map[string]bool{
ExternalSettings: config.ExternalSettings{
PostResolve: config.ExternalMatchers{Exact: map[string]bool{
"/external-file": true,
},
NodeModules: map[string]bool{
}},
PreResolve: config.ExternalMatchers{Exact: map[string]bool{
"external-pkg": true,
"@scope/external-pkg": true,
},
}},
},
},
expectedScanLog: `entry.js: WARNING: "./present-file" should be marked as external for use with "require.resolve"
Expand Down Expand Up @@ -3941,10 +3951,10 @@ func TestInject(t *testing.T) {
"/collision.js",
"/re-export.js",
},
ExternalModules: config.ExternalModules{
NodeModules: map[string]bool{
ExternalSettings: config.ExternalSettings{
PreResolve: config.ExternalMatchers{Exact: map[string]bool{
"external-pkg": true,
},
}},
},
},
})
Expand Down Expand Up @@ -4105,12 +4115,12 @@ func TestInjectImportOrder(t *testing.T) {
"/inject-1.js",
"/inject-2.js",
},
ExternalModules: config.ExternalModules{
NodeModules: map[string]bool{
ExternalSettings: config.ExternalSettings{
PreResolve: config.ExternalMatchers{Exact: map[string]bool{
"first": true,
"second": true,
"third": true,
},
}},
},
},
})
Expand Down Expand Up @@ -4564,10 +4574,10 @@ func TestExternalES6ConvertedToCommonJS(t *testing.T) {
Mode: config.ModeBundle,
AbsOutputFile: "/out.js",
OutputFormat: config.FormatESModule,
ExternalModules: config.ExternalModules{
NodeModules: map[string]bool{
ExternalSettings: config.ExternalSettings{
PreResolve: config.ExternalMatchers{Exact: map[string]bool{
"x": true,
},
}},
},
},
})
Expand Down Expand Up @@ -4794,10 +4804,10 @@ func TestImportNamespaceThisValue(t *testing.T) {
Mode: config.ModeBundle,
OutputFormat: config.FormatCommonJS,
AbsOutputDir: "/out",
ExternalModules: config.ExternalModules{
NodeModules: map[string]bool{
ExternalSettings: config.ExternalSettings{
PreResolve: config.ExternalMatchers{Exact: map[string]bool{
"external": true,
},
}},
},
},
})
Expand Down Expand Up @@ -4842,10 +4852,10 @@ func TestQuotedProperty(t *testing.T) {
Mode: config.ModeBundle,
OutputFormat: config.FormatCommonJS,
AbsOutputDir: "/out",
ExternalModules: config.ExternalModules{
NodeModules: map[string]bool{
ExternalSettings: config.ExternalSettings{
PreResolve: config.ExternalMatchers{Exact: map[string]bool{
"ext": true,
},
}},
},
},
})
Expand All @@ -4865,10 +4875,10 @@ func TestQuotedPropertyMangle(t *testing.T) {
OutputFormat: config.FormatCommonJS,
AbsOutputDir: "/out",
MangleSyntax: true,
ExternalModules: config.ExternalModules{
NodeModules: map[string]bool{
ExternalSettings: config.ExternalSettings{
PreResolve: config.ExternalMatchers{Exact: map[string]bool{
"ext": true,
},
}},
},
},
})
Expand Down Expand Up @@ -4932,10 +4942,10 @@ func TestRequireShimSubstitution(t *testing.T) {
options: config.Options{
Mode: config.ModeBundle,
AbsOutputDir: "/out",
ExternalModules: config.ExternalModules{
NodeModules: map[string]bool{
ExternalSettings: config.ExternalSettings{
PreResolve: config.ExternalMatchers{Exact: map[string]bool{
"some-path": true,
},
}},
},
UnsupportedJSFeatures: compat.DynamicImport,
},
Expand Down Expand Up @@ -5279,10 +5289,10 @@ func TestWarnCommonJSExportsInESMBundle(t *testing.T) {
Mode: config.ModeBundle,
AbsOutputDir: "/out",
OutputFormat: config.FormatCommonJS,
ExternalModules: config.ExternalModules{
NodeModules: map[string]bool{
ExternalSettings: config.ExternalSettings{
PreResolve: config.ExternalMatchers{Exact: map[string]bool{
"bar": true,
},
}},
},
},
expectedScanLog: `cjs-in-esm.js: WARNING: The CommonJS "exports" variable is treated as a global variable in an ECMAScript module and may not work as expected
Expand Down
Loading

0 comments on commit 345f64c

Please sign in to comment.