Skip to content

Commit 0db1b82

Browse files
evanwevanw
committed
fix #3998: avoid outbase in identifier names
1 parent 7236472 commit 0db1b82

File tree

6 files changed

+194
-25
lines changed

6 files changed

+194
-25
lines changed

‎CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,12 @@
6464

6565
This can sometimes expose additional minification opportunities.
6666

67+
* Avoid using the parent directory name for determinism ([#3998](https://github.com/evanw/esbuild/issues/3998))
68+
69+
To make generated code more readable, esbuild includes the name of the source file when generating certain variable names within the file. Specifically bundling a CommonJS file generates a variable to store the lazily-evaluated module initializer. However, if a file is named `index.js` (or with a different extension), esbuild will use the name of the parent directory instead for a better name (since many packages have files all named `index.js` but have unique directory names).
70+
71+
This is problematic when the bundle entry point is named `index.js` and the parent directory name is non-deterministic (e.g. a temporary directory created by a build script). To avoid non-determinism in esbuild's output, esbuild will now use `index` instead of the parent directory in this case. Specifically this will happen if the parent directory is equal to esbuild's `outbase` API option, which defaults to the [lowest common ancestor](https://en.wikipedia.org/wiki/Lowest_common_ancestor) of all user-specified entry point paths.
72+
6773
* Experimental support for esbuild on NetBSD ([#3974](https://github.com/evanw/esbuild/pull/3974))
6874

6975
With this release, esbuild now has a published binary executable for [NetBSD](https://www.netbsd.org/) in the [`@esbuild/netbsd-arm64`](https://www.npmjs.com/package/@esbuild/netbsd-arm64) npm package, and esbuild's installer has been modified to attempt to use it when on NetBSD. Hopefully this makes installing esbuild via npm work on NetBSD. This change was contributed by [@bsiegert](https://github.com/bsiegert).

‎internal/bundler/bundler.go

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -120,11 +120,29 @@ type tlaCheck struct {
120120
}
121121

122122
func parseFile(args parseArgs) {
123+
pathForIdentifierName := args.keyPath.Text
124+
125+
// Identifier name generation may use the name of the parent folder if the
126+
// file name starts with "index". However, this is problematic when the
127+
// parent folder includes the parent directory of what the developer
128+
// considers to be the root of the source tree. If that happens, strip the
129+
// parent folder to avoid including it in the generated name.
130+
if relative, ok := args.fs.Rel(args.options.AbsOutputBase, pathForIdentifierName); ok {
131+
for {
132+
next := strings.TrimPrefix(strings.TrimPrefix(relative, "../"), "..\\")
133+
if relative == next {
134+
break
135+
}
136+
relative = next
137+
}
138+
pathForIdentifierName = relative
139+
}
140+
123141
source := logger.Source{
124142
Index: args.sourceIndex,
125143
KeyPath: args.keyPath,
126144
PrettyPath: args.prettyPath,
127-
IdentifierName: js_ast.GenerateNonUniqueNameFromPath(args.keyPath.Text),
145+
IdentifierName: js_ast.GenerateNonUniqueNameFromPath(pathForIdentifierName),
128146
}
129147

130148
var loader config.Loader
@@ -1857,15 +1875,20 @@ func (s *scanner) addEntryPoints(entryPoints []EntryPoint) []graph.EntryPoint {
18571875
return nil
18581876
}
18591877

1860-
// Parse all entry points that were resolved successfully
1878+
// Determine output paths for all entry points that were resolved successfully
1879+
type entryPointToParse struct {
1880+
index int
1881+
parse func() uint32
1882+
}
1883+
var entryPointsToParse []entryPointToParse
18611884
for i, info := range entryPointInfos {
18621885
if info.results == nil {
18631886
continue
18641887
}
18651888

18661889
for _, resolveResult := range info.results {
1890+
resolveResult := resolveResult
18671891
prettyPath := resolver.PrettyPath(s.fs, resolveResult.PathPair.Primary)
1868-
sourceIndex := s.maybeParseFile(resolveResult, prettyPath, nil, logger.Range{}, nil, inputKindEntryPoint, nil)
18691892
outputPath := entryPoints[i].OutputPath
18701893
outputPathWasAutoGenerated := false
18711894

@@ -1900,9 +1923,17 @@ func (s *scanner) addEntryPoints(entryPoints []EntryPoint) []graph.EntryPoint {
19001923
outputPathWasAutoGenerated = true
19011924
}
19021925

1926+
// Defer parsing for this entry point until later
1927+
entryPointsToParse = append(entryPointsToParse, entryPointToParse{
1928+
index: len(entryMetas),
1929+
parse: func() uint32 {
1930+
return s.maybeParseFile(resolveResult, prettyPath, nil, logger.Range{}, nil, inputKindEntryPoint, nil)
1931+
},
1932+
})
1933+
19031934
entryMetas = append(entryMetas, graph.EntryPoint{
19041935
OutputPath: outputPath,
1905-
SourceIndex: sourceIndex,
1936+
SourceIndex: ast.InvalidRef.SourceIndex,
19061937
OutputPathWasAutoGenerated: outputPathWasAutoGenerated,
19071938
})
19081939
}
@@ -1924,6 +1955,11 @@ func (s *scanner) addEntryPoints(entryPoints []EntryPoint) []graph.EntryPoint {
19241955
}
19251956
}
19261957

1958+
// Only parse entry points after "AbsOutputBase" has been determined
1959+
for _, toParse := range entryPointsToParse {
1960+
entryMetas[toParse.index].SourceIndex = toParse.parse()
1961+
}
1962+
19271963
// Turn all output paths back into relative paths, but this time relative to
19281964
// the "outbase" value we computed above
19291965
for i := range entryMetas {

‎internal/bundler_tests/bundler_default_test.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9123,3 +9123,51 @@ func TestStringExportNamesIIFE(t *testing.T) {
91239123
},
91249124
})
91259125
}
9126+
9127+
func TestSourceIdentifierNameIndexSingleEntry(t *testing.T) {
9128+
default_suite.expectBundled(t, bundled{
9129+
files: map[string]string{
9130+
// Generate identifier names for top-level and nested files
9131+
"/Users/user/project/index.js": `
9132+
require('.')
9133+
require('pkg')
9134+
require('./nested')
9135+
`,
9136+
"/Users/user/project/nested/index.js": `exports.nested = true`,
9137+
"/Users/user/project/node_modules/pkg/index.js": `exports.pkg = true`,
9138+
},
9139+
entryPaths: []string{"/Users/user/project/index.js"},
9140+
options: config.Options{
9141+
Mode: config.ModeBundle,
9142+
AbsOutputDir: "/Users/user/project/out",
9143+
},
9144+
})
9145+
}
9146+
9147+
func TestSourceIdentifierNameIndexMultipleEntry(t *testing.T) {
9148+
default_suite.expectBundled(t, bundled{
9149+
files: map[string]string{
9150+
// Generate identifier names for top-level and nested files
9151+
"/Users/user/project/home/index.js": `
9152+
require('.')
9153+
require('pkg')
9154+
require('../common')
9155+
`,
9156+
"/Users/user/project/about/index.js": `
9157+
require('.')
9158+
require('pkg')
9159+
require('../common')
9160+
`,
9161+
"/Users/user/project/common/index.js": `exports.common = true`,
9162+
"/Users/user/project/node_modules/pkg/index.js": `exports.pkg = true`,
9163+
},
9164+
entryPaths: []string{
9165+
"/Users/user/project/home/index.js",
9166+
"/Users/user/project/about/index.js",
9167+
},
9168+
options: config.Options{
9169+
Mode: config.ModeBundle,
9170+
AbsOutputDir: "/Users/user/project/out",
9171+
},
9172+
})
9173+
}

‎internal/bundler_tests/snapshots/snapshots_default.txt

Lines changed: 83 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5946,23 +5946,23 @@ console.log("cache:", require.cache);
59465946
TestRequireParentDirCommonJS
59475947
---------- /out.js ----------
59485948
// Users/user/project/src/index.js
5949-
var require_src = __commonJS({
5949+
var require_index = __commonJS({
59505950
"Users/user/project/src/index.js"(exports, module) {
59515951
module.exports = 123;
59525952
}
59535953
});
59545954

59555955
// Users/user/project/src/dir/entry.js
5956-
console.log(require_src());
5956+
console.log(require_index());
59575957

59585958
================================================================================
59595959
TestRequireParentDirES6
59605960
---------- /out.js ----------
59615961
// Users/user/project/src/index.js
5962-
var src_default = 123;
5962+
var index_default = 123;
59635963

59645964
// Users/user/project/src/dir/entry.js
5965-
console.log(src_default);
5965+
console.log(index_default);
59665966

59675967
================================================================================
59685968
TestRequirePropertyAccessCommonJS
@@ -6147,6 +6147,85 @@ function fn() {
61476147
// entry.js
61486148
console.log(fn());
61496149

6150+
================================================================================
6151+
TestSourceIdentifierNameIndexMultipleEntry
6152+
---------- /Users/user/project/out/home/index.js ----------
6153+
// Users/user/project/node_modules/pkg/index.js
6154+
var require_pkg = __commonJS({
6155+
"Users/user/project/node_modules/pkg/index.js"(exports) {
6156+
exports.pkg = true;
6157+
}
6158+
});
6159+
6160+
// Users/user/project/common/index.js
6161+
var require_common = __commonJS({
6162+
"Users/user/project/common/index.js"(exports) {
6163+
exports.common = true;
6164+
}
6165+
});
6166+
6167+
// Users/user/project/home/index.js
6168+
var require_home = __commonJS({
6169+
"Users/user/project/home/index.js"() {
6170+
require_home();
6171+
require_pkg();
6172+
require_common();
6173+
}
6174+
});
6175+
export default require_home();
6176+
6177+
---------- /Users/user/project/out/about/index.js ----------
6178+
// Users/user/project/node_modules/pkg/index.js
6179+
var require_pkg = __commonJS({
6180+
"Users/user/project/node_modules/pkg/index.js"(exports) {
6181+
exports.pkg = true;
6182+
}
6183+
});
6184+
6185+
// Users/user/project/common/index.js
6186+
var require_common = __commonJS({
6187+
"Users/user/project/common/index.js"(exports) {
6188+
exports.common = true;
6189+
}
6190+
});
6191+
6192+
// Users/user/project/about/index.js
6193+
var require_about = __commonJS({
6194+
"Users/user/project/about/index.js"() {
6195+
require_about();
6196+
require_pkg();
6197+
require_common();
6198+
}
6199+
});
6200+
export default require_about();
6201+
6202+
================================================================================
6203+
TestSourceIdentifierNameIndexSingleEntry
6204+
---------- /Users/user/project/out/index.js ----------
6205+
// Users/user/project/node_modules/pkg/index.js
6206+
var require_pkg = __commonJS({
6207+
"Users/user/project/node_modules/pkg/index.js"(exports) {
6208+
exports.pkg = true;
6209+
}
6210+
});
6211+
6212+
// Users/user/project/nested/index.js
6213+
var require_nested = __commonJS({
6214+
"Users/user/project/nested/index.js"(exports) {
6215+
exports.nested = true;
6216+
}
6217+
});
6218+
6219+
// Users/user/project/index.js
6220+
var require_index = __commonJS({
6221+
"Users/user/project/index.js"() {
6222+
require_index();
6223+
require_pkg();
6224+
require_nested();
6225+
}
6226+
});
6227+
export default require_index();
6228+
61506229
================================================================================
61516230
TestSourceMap
61526231
---------- /Users/user/project/out.js.map ----------

‎internal/bundler_tests/snapshots/snapshots_packagejson.txt

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -683,10 +683,10 @@ TestPackageJsonImportSelfUsingImport
683683
var foo_import_default = "foo";
684684

685685
// Users/user/project/src/index.js
686-
var src_default = "index";
687-
console.log(src_default, foo_import_default);
686+
var index_default = "index";
687+
console.log(index_default, foo_import_default);
688688
export {
689-
src_default as default
689+
index_default as default
690690
};
691691

692692
================================================================================
@@ -696,10 +696,10 @@ TestPackageJsonImportSelfUsingImportScoped
696696
var foo_import_default = "foo";
697697

698698
// Users/user/project/src/index.js
699-
var src_default = "index";
700-
console.log(src_default, foo_import_default);
699+
var index_default = "index";
700+
console.log(index_default, foo_import_default);
701701
export {
702-
src_default as default
702+
index_default as default
703703
};
704704

705705
================================================================================
@@ -713,16 +713,16 @@ var require_foo_require = __commonJS({
713713
});
714714

715715
// Users/user/project/src/index.js
716-
var require_src = __commonJS({
716+
var require_index = __commonJS({
717717
"Users/user/project/src/index.js"(exports, module) {
718718
module.exports = "index";
719719
console.log(
720-
require_src(),
720+
require_index(),
721721
require_foo_require()
722722
);
723723
}
724724
});
725-
export default require_src();
725+
export default require_index();
726726

727727
================================================================================
728728
TestPackageJsonImportSelfUsingRequireScoped
@@ -735,16 +735,16 @@ var require_foo_require = __commonJS({
735735
});
736736

737737
// Users/user/project/src/index.js
738-
var require_src = __commonJS({
738+
var require_index = __commonJS({
739739
"Users/user/project/src/index.js"(exports, module) {
740740
module.exports = "index";
741741
console.log(
742-
require_src(),
742+
require_index(),
743743
require_foo_require()
744744
);
745745
}
746746
});
747-
export default require_src();
747+
export default require_index();
748748

749749
================================================================================
750750
TestPackageJsonImports

‎internal/bundler_tests/snapshots/snapshots_splitting.txt

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,23 +25,23 @@ var init_a = __esm({
2525
var B;
2626
var init_b = __esm({
2727
"src/b.js"() {
28-
B = async () => (await Promise.resolve().then(() => (init_src(), src_exports))).A;
28+
B = async () => (await Promise.resolve().then(() => (init_index(), index_exports))).A;
2929
}
3030
});
3131

3232
// src/index.js
33-
var src_exports = {};
34-
__export(src_exports, {
33+
var index_exports = {};
34+
__export(index_exports, {
3535
A: () => A,
3636
B: () => B
3737
});
38-
var init_src = __esm({
38+
var init_index = __esm({
3939
"src/index.js"() {
4040
init_a();
4141
init_b();
4242
}
4343
});
44-
init_src();
44+
init_index();
4545
export {
4646
A,
4747
B

0 commit comments

Comments
 (0)