Skip to content

Commit 22a9cf5

Browse files
evanwevanw
committed
fix #3341, fix #3608: sort .ts right after .js 
1 parent f8ec300 commit 22a9cf5

4 files changed

Lines changed: 110 additions & 8 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,15 @@
11
# Changelog
22

3+
## Unreleased
4+
5+
* Reorder implicit file extensions within `node_modules` ([#3341](https://github.com/evanw/esbuild/issues/3341), [#3608](https://github.com/evanw/esbuild/issues/3608))
6+
7+
In [version 0.18.0](https://github.com/evanw/esbuild/releases/v0.18.0), esbuild changed the behavior of implicit file extensions within `node_modules` directories (i.e. in published packages) to prefer `.js` over `.ts` even when the `--resolve-extensions=` order prefers `.ts` over `.js` (which it does by default). However, doing that also accidentally made esbuild prefer `.css` over `.ts`, which caused problems for people that published packages containing both TypeScript and CSS in files with the same name.
8+
9+
With this release, esbuild will reorder TypeScript file extensions immediately after the last JavaScript file extensions in the implicit file extension order instead of putting them at the end of the order. Specifically the default implicit file extension order is `.tsx,.ts,.jsx,.js,.css,.json` which used to become `.jsx,.js,.css,.json,.tsx,.ts` in `node_modules` directories. With this release it will now become `.jsx,.js,.tsx,.ts,.css,.json` instead.
10+
11+
Why even rewrite the implicit file extension order at all? One reason is because the `.js` file is more likely to behave correctly than the `.ts` file. The behavior of the `.ts` file may depend on `tsconfig.json` and the `tsconfig.json` file may not even be published, or may use `extends` to refer to a base `tsconfig.json` file that wasn't published. People can get into this situation when they forget to add all `.ts` files to their `.npmignore` file before publishing to npm. Picking `.js` over `.ts` helps make it more likely that resulting bundle will behave correctly.
12+
313
## 0.19.12
414

515
* The "preserve" JSX mode now preserves JSX text verbatim ([#3605](https://github.com/evanw/esbuild/issues/3605))

‎internal/bundler_tests/bundler_ts_test.go‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2934,3 +2934,49 @@ func TestTSPrintNonFiniteNumberInsideWith(t *testing.T) {
29342934
},
29352935
})
29362936
}
2937+
2938+
func TestTSImportInNodeModulesNameCollisionWithCSS(t *testing.T) {
2939+
ts_suite.expectBundled(t, bundled{
2940+
files: map[string]string{
2941+
"/entry.ts": `
2942+
import "pkg"
2943+
`,
2944+
"/node_modules/pkg/index.ts": `
2945+
import js_ts_css from "./js_ts_css"
2946+
import ts_css from "./ts_css"
2947+
import js_ts from "./js_ts"
2948+
js_ts_css()
2949+
ts_css()
2950+
js_ts()
2951+
`,
2952+
"/node_modules/pkg/js_ts_css.js": `
2953+
import './js_ts_css.css'
2954+
export default function() {}
2955+
`,
2956+
"/node_modules/pkg/js_ts_css.ts": `
2957+
TEST FAILED
2958+
`,
2959+
"/node_modules/pkg/js_ts_css.css": `
2960+
.js_ts_css {}
2961+
`,
2962+
"/node_modules/pkg/ts_css.ts": `
2963+
import './ts_css.css'
2964+
export default function() {}
2965+
`,
2966+
"/node_modules/pkg/ts_css.css": `
2967+
.ts_css {}
2968+
`,
2969+
"/node_modules/pkg/js_ts.js": `
2970+
export default function() {}
2971+
`,
2972+
"/node_modules/pkg/js_ts.ts": `
2973+
TEST FAILED
2974+
`,
2975+
},
2976+
entryPaths: []string{"/entry.ts"},
2977+
options: config.Options{
2978+
Mode: config.ModeBundle,
2979+
AbsOutputFile: "/out.js",
2980+
},
2981+
})
2982+
}

‎internal/bundler_tests/snapshots/snapshots_ts.txt‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1464,6 +1464,35 @@ var value_copy = value;
14641464
var foo = value_copy;
14651465
console.log(foo);
14661466

1467+
================================================================================
1468+
TestTSImportInNodeModulesNameCollisionWithCSS
1469+
---------- /out.js ----------
1470+
// node_modules/pkg/js_ts_css.js
1471+
function js_ts_css_default() {
1472+
}
1473+
1474+
// node_modules/pkg/ts_css.ts
1475+
function ts_css_default() {
1476+
}
1477+
1478+
// node_modules/pkg/js_ts.js
1479+
function js_ts_default() {
1480+
}
1481+
1482+
// node_modules/pkg/index.ts
1483+
js_ts_css_default();
1484+
ts_css_default();
1485+
js_ts_default();
1486+
1487+
---------- /out.css ----------
1488+
/* node_modules/pkg/js_ts_css.css */
1489+
.js_ts_css {
1490+
}
1491+
1492+
/* node_modules/pkg/ts_css.css */
1493+
.ts_css {
1494+
}
1495+
14671496
================================================================================
14681497
TestTSImportMTS
14691498
---------- /out.js ----------

‎internal/resolver/resolver.go‎

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -248,17 +248,34 @@ func NewResolver(call config.APICall, fs fs.FS, log logger.Log, caches *cache.Ca
248248
}
249249
}
250250

251-
// Sort all JavaScript file extensions after TypeScript file extensions
252-
// for imports of files inside of "node_modules" directories
251+
// Sort all TypeScript file extensions after all JavaScript file extensions
252+
// for imports of files inside of "node_modules" directories. But insert
253+
// the TypeScript file extensions right after the last JavaScript file
254+
// extension instead of at the end so that they might come before the
255+
// first CSS file extension, which is important to people that publish
256+
// TypeScript and CSS code to npm with the same file names for both.
253257
nodeModulesExtensionOrder := make([]string, 0, len(options.ExtensionOrder))
254-
for _, ext := range options.ExtensionOrder {
255-
if loader, ok := options.ExtensionToLoader[ext]; !ok || !loader.IsTypeScript() {
256-
nodeModulesExtensionOrder = append(nodeModulesExtensionOrder, ext)
258+
split := 0
259+
for i, ext := range options.ExtensionOrder {
260+
if loader, ok := options.ExtensionToLoader[ext]; ok && loader == config.LoaderJS || loader == config.LoaderJSX {
261+
split = i + 1 // Split after the last JavaScript extension
257262
}
258263
}
259-
for _, ext := range options.ExtensionOrder {
260-
if loader, ok := options.ExtensionToLoader[ext]; ok && loader.IsTypeScript() {
261-
nodeModulesExtensionOrder = append(nodeModulesExtensionOrder, ext)
264+
if split != 0 { // Only do this if there are any JavaScript extensions
265+
for _, ext := range options.ExtensionOrder[:split] { // Non-TypeScript extensions before the split
266+
if loader, ok := options.ExtensionToLoader[ext]; !ok || !loader.IsTypeScript() {
267+
nodeModulesExtensionOrder = append(nodeModulesExtensionOrder, ext)
268+
}
269+
}
270+
for _, ext := range options.ExtensionOrder { // All TypeScript extensions
271+
if loader, ok := options.ExtensionToLoader[ext]; ok && loader.IsTypeScript() {
272+
nodeModulesExtensionOrder = append(nodeModulesExtensionOrder, ext)
273+
}
274+
}
275+
for _, ext := range options.ExtensionOrder[split:] { // Non-TypeScript extensions after the split
276+
if loader, ok := options.ExtensionToLoader[ext]; !ok || !loader.IsTypeScript() {
277+
nodeModulesExtensionOrder = append(nodeModulesExtensionOrder, ext)
278+
}
262279
}
263280
}
264281

0 commit comments

Comments
 (0)