Skip to content

Commit

Permalink
fix #3319: missing symbol usage in glob transform
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Dec 18, 2023
1 parent 461ca73 commit e1b7050
Show file tree
Hide file tree
Showing 5 changed files with 248 additions and 54 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

* Fix glob imports in TypeScript files ([#3319](https://github.com/evanw/esbuild/issues/3319))

This release fixes a problem where bundling a TypeScript file containing a glob import could emit a call to a helper function that doesn't exist. The problem happened because esbuild's TypeScript transformation removes unused imports (which is required for correctness, as they may be type-only imports) and esbuild's glob import transformation wasn't correctly marking the imported helper function as used. This wasn't caught earlier because most of esbuild's glob import tests were written in JavaScript, not in TypeScript.

* Fix a panic when transforming optional chaining with `define` ([#3551](https://github.com/evanw/esbuild/issues/3551), [#3554](https://github.com/evanw/esbuild/pull/3554))

This release fixes a case where esbuild could crash with a panic, which was triggered by using `define` to replace an expression containing an optional chain. Here is an example:
Expand Down
55 changes: 55 additions & 0 deletions internal/bundler_tests/bundler_glob_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,33 @@ func TestGlobBasicNoSplitting(t *testing.T) {
})
}

func TestTSGlobBasicNoSplitting(t *testing.T) {
glob_suite.expectBundled(t, bundled{
files: map[string]string{
"/entry.ts": `
const ab = Math.random() < 0.5 ? 'a.ts' : 'b.ts'
console.log({
concat: {
require: require('./src/' + ab),
import: import('./src/' + ab),
},
template: {
require: require(` + "`./src/${ab}`" + `),
import: import(` + "`./src/${ab}`" + `),
},
})
`,
"/src/a.ts": `module.exports = 'a'`,
"/src/b.ts": `module.exports = 'b'`,
},
entryPaths: []string{"/entry.ts"},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputFile: "/out.js",
},
})
}

func TestGlobBasicSplitting(t *testing.T) {
glob_suite.expectBundled(t, bundled{
files: map[string]string{
Expand Down Expand Up @@ -65,6 +92,34 @@ func TestGlobBasicSplitting(t *testing.T) {
})
}

func TestTSGlobBasicSplitting(t *testing.T) {
glob_suite.expectBundled(t, bundled{
files: map[string]string{
"/entry.ts": `
const ab = Math.random() < 0.5 ? 'a.ts' : 'b.ts'
console.log({
concat: {
require: require('./src/' + ab),
import: import('./src/' + ab),
},
template: {
require: require(` + "`./src/${ab}`" + `),
import: import(` + "`./src/${ab}`" + `),
},
})
`,
"/src/a.ts": `module.exports = 'a'`,
"/src/b.ts": `module.exports = 'b'`,
},
entryPaths: []string{"/entry.ts"},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputDir: "/out",
CodeSplitting: true,
},
})
}

func TestGlobDirDoesNotExist(t *testing.T) {
glob_suite.expectBundled(t, bundled{
files: map[string]string{
Expand Down
132 changes: 132 additions & 0 deletions internal/bundler_tests/snapshots/snapshots_glob.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -258,3 +258,135 @@ console.log({
import: globImport_src_js(`./src/${ab}.js`)
}
});

================================================================================
TestTSGlobBasicNoSplitting
---------- /out.js ----------
// src/a.ts
var require_a = __commonJS({
"src/a.ts"(exports, module) {
module.exports = "a";
}
});

// src/b.ts
var require_b = __commonJS({
"src/b.ts"(exports, module) {
module.exports = "b";
}
});

// require("./src/**/*") in entry.ts
var globRequire_src = __glob({
"./src/a.ts": () => require_a(),
"./src/b.ts": () => require_b()
});

// import("./src/**/*") in entry.ts
var globImport_src = __glob({
"./src/a.ts": () => Promise.resolve().then(() => __toESM(require_a())),
"./src/b.ts": () => Promise.resolve().then(() => __toESM(require_b()))
});

// entry.ts
var ab = Math.random() < 0.5 ? "a.ts" : "b.ts";
console.log({
concat: {
require: globRequire_src("./src/" + ab),
import: globImport_src("./src/" + ab)
},
template: {
require: globRequire_src(`./src/${ab}`),
import: globImport_src(`./src/${ab}`)
}
});

================================================================================
TestTSGlobBasicSplitting
---------- /out/entry.js ----------
import {
require_a
} from "./chunk-NW3BCH2J.js";
import {
require_b
} from "./chunk-LX6LSYGO.js";
import {
__glob
} from "./chunk-NI2FVOS3.js";

// require("./src/**/*") in entry.ts
var globRequire_src = __glob({
"./src/a.ts": () => require_a(),
"./src/b.ts": () => require_b()
});

// import("./src/**/*") in entry.ts
var globImport_src = __glob({
"./src/a.ts": () => import("./a-CIKLEGQA.js"),
"./src/b.ts": () => import("./b-UWTMLRMB.js")
});

// entry.ts
var ab = Math.random() < 0.5 ? "a.ts" : "b.ts";
console.log({
concat: {
require: globRequire_src("./src/" + ab),
import: globImport_src("./src/" + ab)
},
template: {
require: globRequire_src(`./src/${ab}`),
import: globImport_src(`./src/${ab}`)
}
});

---------- /out/a-CIKLEGQA.js ----------
import {
require_a
} from "./chunk-NW3BCH2J.js";
import "./chunk-NI2FVOS3.js";
export default require_a();

---------- /out/chunk-NW3BCH2J.js ----------
import {
__commonJS
} from "./chunk-NI2FVOS3.js";

// src/a.ts
var require_a = __commonJS({
"src/a.ts"(exports, module) {
module.exports = "a";
}
});

export {
require_a
};

---------- /out/b-UWTMLRMB.js ----------
import {
require_b
} from "./chunk-LX6LSYGO.js";
import "./chunk-NI2FVOS3.js";
export default require_b();

---------- /out/chunk-LX6LSYGO.js ----------
import {
__commonJS
} from "./chunk-NI2FVOS3.js";

// src/b.ts
var require_b = __commonJS({
"src/b.ts"(exports, module) {
module.exports = "b";
}
});

export {
require_b
};

---------- /out/chunk-NI2FVOS3.js ----------
export {
__glob,
__commonJS
};
1 change: 1 addition & 0 deletions internal/js_parser/js_parser.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15762,6 +15762,7 @@ outer:
})
}

p.recordUsage(ref)
return js_ast.Expr{Loc: expr.Loc, Data: &js_ast.ECall{
Target: js_ast.Expr{Loc: expr.Loc, Data: &js_ast.EIdentifier{Ref: ref}},
Args: []js_ast.Expr{expr},
Expand Down
110 changes: 56 additions & 54 deletions scripts/end-to-end-tests.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8214,66 +8214,68 @@ if (process.platform === 'darwin' || process.platform === 'win32') {
}

// Test glob import behavior
tests.push(
test(['./src/*.ts', '--outdir=out', '--bundle', '--format=cjs'], {
'node.js': `
if (require('./out/a.js') !== 10) throw 'fail: a'
if (require('./out/b.js') !== 11) throw 'fail: b'
if (require('./out/c.js') !== 12) throw 'fail: c'
`,
'src/a.ts': `module.exports = 10 as number`,
'src/b.ts': `module.exports = 11 as number`,
'src/c.ts': `module.exports = 12 as number`,
}),
test(['in.js', '--outfile=node.js', '--bundle'], {
'in.js': `
for (let i = 0; i < 3; i++) {
const value = require('./' + i + '.js')
if (value !== i + 10) throw 'fail: ' + i
}
`,
'0.js': `module.exports = 10`,
'1.js': `module.exports = 11`,
'2.js': `module.exports = 12`,
}),
test(['in.js', '--outfile=node.js', '--bundle'], {
'in.js': `
for (let i = 0; i < 3; i++) {
const value = require(\`./\${i}.js\`)
if (value !== i + 10) throw 'fail: ' + i
}
`,
'0.js': `module.exports = 10`,
'1.js': `module.exports = 11`,
'2.js': `module.exports = 12`,
}),
test(['in.js', '--outfile=node.js', '--bundle'], {
'in.js': `
export let async = async () => {
for (const ext of ['.js', '.ts']) {
tests.push(
test(['./src/*' + ext, '--outdir=out', '--bundle', '--format=cjs'], {
'node.js': `
if (require('./out/a.js') !== 10) throw 'fail: a'
if (require('./out/b.js') !== 11) throw 'fail: b'
if (require('./out/c.js') !== 12) throw 'fail: c'
`,
['src/a' + ext]: `module.exports = 10`,
['src/b' + ext]: `module.exports = 11`,
['src/c' + ext]: `module.exports = 12`,
}),
test(['in' + ext, '--outfile=node.js', '--bundle'], {
['in' + ext]: `
for (let i = 0; i < 3; i++) {
const { default: value } = await import('./' + i + '.js')
const value = require('./' + i + '${ext}')
if (value !== i + 10) throw 'fail: ' + i
}
}
`,
'0.js': `export default 10`,
'1.js': `export default 11`,
'2.js': `export default 12`,
}, { async: true }),
test(['in.js', '--outfile=node.js', '--bundle'], {
'in.js': `
export let async = async () => {
`,
['0' + ext]: `module.exports = 10`,
['1' + ext]: `module.exports = 11`,
['2' + ext]: `module.exports = 12`,
}),
test(['in' + ext, '--outfile=node.js', '--bundle'], {
['in' + ext]: `
for (let i = 0; i < 3; i++) {
const { default: value } = await import(\`./\${i}.js\`)
const value = require(\`./\${i}${ext}\`)
if (value !== i + 10) throw 'fail: ' + i
}
}
`,
'0.js': `export default 10`,
'1.js': `export default 11`,
'2.js': `export default 12`,
}, { async: true }),
)
`,
['0' + ext]: `module.exports = 10`,
['1' + ext]: `module.exports = 11`,
['2' + ext]: `module.exports = 12`,
}),
test(['in' + ext, '--outfile=node.js', '--bundle'], {
['in' + ext]: `
export let async = async () => {
for (let i = 0; i < 3; i++) {
const { default: value } = await import('./' + i + '${ext}')
if (value !== i + 10) throw 'fail: ' + i
}
}
`,
['0' + ext]: `export default 10`,
['1' + ext]: `export default 11`,
['2' + ext]: `export default 12`,
}, { async: true }),
test(['in' + ext, '--outfile=node.js', '--bundle'], {
['in' + ext]: `
export let async = async () => {
for (let i = 0; i < 3; i++) {
const { default: value } = await import(\`./\${i}${ext}\`)
if (value !== i + 10) throw 'fail: ' + i
}
}
`,
['0' + ext]: `export default 10`,
['1' + ext]: `export default 11`,
['2' + ext]: `export default 12`,
}, { async: true }),
)
}

// Test "using" declarations
for (const flags of [[], '--supported:async-await=false']) {
Expand Down

0 comments on commit e1b7050

Please sign in to comment.