Skip to content

Commit 4cdf03c

Browse files
committed
fix #4053: reordering of .tsx in node_modules
1 parent dc71977 commit 4cdf03c

File tree

6 files changed

+87
-30
lines changed

6 files changed

+87
-30
lines changed

CHANGELOG.md

+4
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,10 @@
162162

163163
With this release, you can now use BigInt literals as define values, such as with `--define:FOO=123n`. Previously trying to do this resulted in a syntax error.
164164

165+
* Fix a bug with resolve extensions in `node_modules` ([#4053](https://github.com/evanw/esbuild/issues/4053))
166+
167+
The `--resolve-extensions=` option lets you specify the order in which to try resolving implicit file extensions. For complicated reasons, esbuild reorders TypeScript file extensions after JavaScript ones inside of `node_modules` so that JavaScript source code is always preferred to TypeScript source code inside of dependencies. However, this reordering had a bug that could accidentally change the relative order of TypeScript file extensions if one of them was a prefix of the other. That bug has been fixed in this release. You can see the issue for details.
168+
165169
* Emit `/* @__KEY__ */` for string literals derived from property names ([#4034](https://github.com/evanw/esbuild/issues/4034))
166170

167171
Property name mangling is an advanced feature that shortens certain property names for better minification (I say "advanced feature" because it's very easy to break your code with it). Sometimes you need to store a property name in a string, such as `obj.get('foo')` instead of `obj.foo`. JavaScript minifiers such as esbuild and [Terser](https://terser.org/) have a convention where a `/* @__KEY__ */` comment before the string makes it behave like a property name. So `obj.get(/* @__KEY__ */ 'foo')` allows the contents of the string `'foo'` to be shortened.

internal/bundler/bundler.go

+1-25
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ func parseFile(args parseArgs) {
190190

191191
// The special "default" loader determines the loader from the file path
192192
if loader == config.LoaderDefault {
193-
loader = loaderFromFileExtension(args.options.ExtensionToLoader, base+ext)
193+
loader = config.LoaderFromFileExtension(args.options.ExtensionToLoader, base+ext)
194194
}
195195

196196
// Reject unsupported import attributes when the loader isn't "copy" (since
@@ -1199,30 +1199,6 @@ func runOnLoadPlugins(
11991199
return loaderPluginResult{loader: config.LoaderNone}, true
12001200
}
12011201

1202-
func loaderFromFileExtension(extensionToLoader map[string]config.Loader, base string) config.Loader {
1203-
// Pick the loader with the longest matching extension. So if there's an
1204-
// extension for ".css" and for ".module.css", we want to match the one for
1205-
// ".module.css" before the one for ".css".
1206-
if i := strings.IndexByte(base, '.'); i != -1 {
1207-
for {
1208-
if loader, ok := extensionToLoader[base[i:]]; ok {
1209-
return loader
1210-
}
1211-
base = base[i+1:]
1212-
i = strings.IndexByte(base, '.')
1213-
if i == -1 {
1214-
break
1215-
}
1216-
}
1217-
} else {
1218-
// If there's no extension, explicitly check for an extensionless loader
1219-
if loader, ok := extensionToLoader[""]; ok {
1220-
return loader
1221-
}
1222-
}
1223-
return config.LoaderNone
1224-
}
1225-
12261202
// Identify the path by its lowercase absolute path name with Windows-specific
12271203
// slashes substituted for standard slashes. This should hopefully avoid path
12281204
// issues on Windows where multiple different paths can refer to the same

internal/bundler_tests/bundler_default_test.go

+40
Original file line numberDiff line numberDiff line change
@@ -9206,3 +9206,43 @@ func TestSourceIdentifierNameIndexMultipleEntry(t *testing.T) {
92069206
},
92079207
})
92089208
}
9209+
9210+
func TestResolveExtensionsOrderIssue4053(t *testing.T) {
9211+
css_suite.expectBundled(t, bundled{
9212+
files: map[string]string{
9213+
"/entry.js": `
9214+
import test from "./Test"
9215+
import image from "expo-image"
9216+
console.log(test === 'Test.web.tsx')
9217+
console.log(image === 'Image.web.tsx')
9218+
`,
9219+
"/Test.web.tsx": `export default 'Test.web.tsx'`,
9220+
"/Test.tsx": `export default 'Test.tsx'`,
9221+
"/node_modules/expo-image/index.js": `
9222+
export { default } from "./Image"
9223+
`,
9224+
"/node_modules/expo-image/Image.web.tsx": `export default 'Image.web.tsx'`,
9225+
"/node_modules/expo-image/Image.tsx": `export default 'Image.tsx'`,
9226+
},
9227+
entryPaths: []string{"/entry.js"},
9228+
options: config.Options{
9229+
Mode: config.ModeBundle,
9230+
AbsOutputFile: "/out.js",
9231+
ExtensionOrder: []string{
9232+
".web.mjs",
9233+
".mjs",
9234+
".web.js",
9235+
".js",
9236+
".web.mts",
9237+
".mts",
9238+
".web.ts",
9239+
".ts",
9240+
".web.jsx",
9241+
".jsx",
9242+
".web.tsx",
9243+
".tsx",
9244+
".json",
9245+
},
9246+
},
9247+
})
9248+
}

internal/bundler_tests/snapshots/snapshots_css.txt

+13
Original file line numberDiff line numberDiff line change
@@ -3443,6 +3443,19 @@ c {
34433443
background: url(data:image/png;base64,Yy0z);
34443444
}
34453445

3446+
================================================================================
3447+
TestResolveExtensionsOrderIssue4053
3448+
---------- /out.js ----------
3449+
// Test.web.tsx
3450+
var Test_web_default = "Test.web.tsx";
3451+
3452+
// node_modules/expo-image/Image.web.tsx
3453+
var Image_web_default = "Image.web.tsx";
3454+
3455+
// entry.js
3456+
console.log(Test_web_default === "Test.web.tsx");
3457+
console.log(Image_web_default === "Image.web.tsx");
3458+
34463459
================================================================================
34473460
TestTextImportURLInCSSText
34483461
---------- /out/entry.css ----------

internal/config/config.go

+24
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,30 @@ func (loader Loader) CanHaveSourceMap() bool {
256256
return false
257257
}
258258

259+
func LoaderFromFileExtension(extensionToLoader map[string]Loader, base string) Loader {
260+
// Pick the loader with the longest matching extension. So if there's an
261+
// extension for ".css" and for ".module.css", we want to match the one for
262+
// ".module.css" before the one for ".css".
263+
if i := strings.IndexByte(base, '.'); i != -1 {
264+
for {
265+
if loader, ok := extensionToLoader[base[i:]]; ok {
266+
return loader
267+
}
268+
base = base[i+1:]
269+
i = strings.IndexByte(base, '.')
270+
if i == -1 {
271+
break
272+
}
273+
}
274+
} else {
275+
// If there's no extension, explicitly check for an extensionless loader
276+
if loader, ok := extensionToLoader[""]; ok {
277+
return loader
278+
}
279+
}
280+
return LoaderNone
281+
}
282+
259283
type Format uint8
260284

261285
const (

internal/resolver/resolver.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ func NewResolver(call config.APICall, fs fs.FS, log logger.Log, caches *cache.Ca
243243
// Filter out non-CSS extensions for CSS "@import" imports
244244
cssExtensionOrder := make([]string, 0, len(options.ExtensionOrder))
245245
for _, ext := range options.ExtensionOrder {
246-
if loader, ok := options.ExtensionToLoader[ext]; !ok || loader.IsCSS() {
246+
if loader := config.LoaderFromFileExtension(options.ExtensionToLoader, ext); loader == config.LoaderNone || loader.IsCSS() {
247247
cssExtensionOrder = append(cssExtensionOrder, ext)
248248
}
249249
}
@@ -257,23 +257,23 @@ func NewResolver(call config.APICall, fs fs.FS, log logger.Log, caches *cache.Ca
257257
nodeModulesExtensionOrder := make([]string, 0, len(options.ExtensionOrder))
258258
split := 0
259259
for i, ext := range options.ExtensionOrder {
260-
if loader, ok := options.ExtensionToLoader[ext]; ok && loader == config.LoaderJS || loader == config.LoaderJSX {
260+
if loader := config.LoaderFromFileExtension(options.ExtensionToLoader, ext); loader == config.LoaderJS || loader == config.LoaderJSX {
261261
split = i + 1 // Split after the last JavaScript extension
262262
}
263263
}
264264
if split != 0 { // Only do this if there are any JavaScript extensions
265265
for _, ext := range options.ExtensionOrder[:split] { // Non-TypeScript extensions before the split
266-
if loader, ok := options.ExtensionToLoader[ext]; !ok || !loader.IsTypeScript() {
266+
if loader := config.LoaderFromFileExtension(options.ExtensionToLoader, ext); !loader.IsTypeScript() {
267267
nodeModulesExtensionOrder = append(nodeModulesExtensionOrder, ext)
268268
}
269269
}
270270
for _, ext := range options.ExtensionOrder { // All TypeScript extensions
271-
if loader, ok := options.ExtensionToLoader[ext]; ok && loader.IsTypeScript() {
271+
if loader := config.LoaderFromFileExtension(options.ExtensionToLoader, ext); loader.IsTypeScript() {
272272
nodeModulesExtensionOrder = append(nodeModulesExtensionOrder, ext)
273273
}
274274
}
275275
for _, ext := range options.ExtensionOrder[split:] { // Non-TypeScript extensions after the split
276-
if loader, ok := options.ExtensionToLoader[ext]; !ok || !loader.IsTypeScript() {
276+
if loader := config.LoaderFromFileExtension(options.ExtensionToLoader, ext); !loader.IsTypeScript() {
277277
nodeModulesExtensionOrder = append(nodeModulesExtensionOrder, ext)
278278
}
279279
}

0 commit comments

Comments
 (0)