Skip to content

Commit

Permalink
fix #617: do not export symbols inside cjs wrapper
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Dec 29, 2020
1 parent 442f2a7 commit 36bb205
Show file tree
Hide file tree
Showing 4 changed files with 119 additions and 0 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@

You can now pass `--sources-content=false` to omit the `sourcesContent` field from generated source maps. The field embeds the original source code inline in the source map and is the largest part of the source map. This is useful if you don't need the original source code and would like a smaller source map (e.g. you only care about stack traces and don't need the source code for debugging).

* Fix exports from ESM files converted to CJS during code splitting ([#617](https://github.com/evanw/esbuild/issues/617))

This release fixes an edge case where files in ECMAScript module format that are converted to CommonJS format during bundling can generate exports to non-top-level symbols when code splitting is active. These files must be converted to CommonJS format if they are referenced by a `require()` call. When that happens, the symbols in that file are placed inside the CommonJS wrapper closure and are no longer top-level symbols. This means they should no longer be considered exportable for cross-chunk export generation due to code splitting. The result of this fix is that these cases no longer generate output files with module instantiation errors.

## 0.8.26

* Ensure the current working directory remains unique per `startService()` call
Expand Down
41 changes: 41 additions & 0 deletions internal/bundler/bundler_splitting_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -484,3 +484,44 @@ func TestSplittingMinifyIdentifiersCrashIssue437(t *testing.T) {
},
})
}

func TestSplittingHybridESMAndCJSIssue617(t *testing.T) {
splitting_suite.expectBundled(t, bundled{
files: map[string]string{
"/a.js": `
export let foo
`,
"/b.js": `
export let bar = require('./a')
`,
},
entryPaths: []string{"/a.js", "/b.js"},
options: config.Options{
Mode: config.ModeBundle,
CodeSplitting: true,
OutputFormat: config.FormatESModule,
AbsOutputDir: "/out",
},
})
}

func TestSplittingHybridCJSAndESMIssue617(t *testing.T) {
splitting_suite.expectBundled(t, bundled{
files: map[string]string{
"/a.js": `
export let foo
exports.bar = 123
`,
"/b.js": `
export {foo} from './a'
`,
},
entryPaths: []string{"/a.js", "/b.js"},
options: config.Options{
Mode: config.ModeBundle,
CodeSplitting: true,
OutputFormat: config.FormatESModule,
AbsOutputDir: "/out",
},
})
}
4 changes: 4 additions & 0 deletions internal/bundler/linker.go
Original file line number Diff line number Diff line change
Expand Up @@ -794,6 +794,10 @@ func (c *linkerContext) computeCrossChunkDependencies(chunks []chunkInfo) {
if importToBind, ok := repr.meta.importsToBind[ref]; ok {
ref = importToBind.ref
symbol = c.symbols.Get(ref)
} else if repr.meta.cjsWrap && ref != repr.ast.WrapperRef {
// The only internal symbol that wrapped CommonJS files export
// is the wrapper itself.
continue
}

// If this is an ES6 import from a CommonJS file, it will become a
Expand Down
70 changes: 70 additions & 0 deletions internal/bundler/snapshots/snapshots_splitting.txt
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,76 @@ import("../node_modules/package/index.js");
// Users/user/project/node_modules/package/index.js
console.log("imported");

================================================================================
TestSplittingHybridCJSAndESMIssue617
---------- /out/a.js ----------
import {
require_a
} from "./chunk.UM5LDUBH.js";
export default require_a();

---------- /out/b.js ----------
import {
__defProp,
__markAsModule,
require_a
} from "./chunk.UM5LDUBH.js";

// b.js
var a = __toModule(require_a());
var export_foo = a.foo;
export {
export_foo as foo
};

---------- /out/chunk.UM5LDUBH.js ----------
// a.js
var require_a = __commonJS((exports) => {
__export(exports, {
foo: () => foo
});
var foo;
exports.bar = 123;
});

export {
__defProp,
require_a,
__markAsModule
};

================================================================================
TestSplittingHybridESMAndCJSIssue617
---------- /out/a.js ----------
import {
require_a
} from "./chunk.YOK3FHV5.js";
export default require_a();

---------- /out/b.js ----------
import {
require_a
} from "./chunk.YOK3FHV5.js";

// b.js
var bar = require_a();
export {
bar
};

---------- /out/chunk.YOK3FHV5.js ----------
// a.js
var require_a = __commonJS((exports) => {
__export(exports, {
foo: () => foo
});
var foo;
});

export {
require_a
};

================================================================================
TestSplittingMinifyIdentifiersCrashIssue437
---------- /out/a.js ----------
Expand Down

0 comments on commit 36bb205

Please sign in to comment.