implement lazy evaluation of esm code

CHANGELOG.md

@@ -1,5 +1,58 @@
 # Changelog
 
+## Breaking Changes
+
+* Change how `require()` and `import()` of ESM works ([#667](https://github.com/evanw/esbuild/issues/667), [#706](https://github.com/evanw/esbuild/issues/706))
+
+    Previously if you call `require()` on an ESM file, or call `import()` on an ESM file with code splitting disabled, esbuild would convert the ESM file to CommonJS. For example, if you had the following input files:
+
+    ```js
+    // cjs-file.js
+    console.log(require('./esm-file.js').foo)
+
+    // esm-file.js
+    export let foo = bar()
+    ```
+
+    The previous bundling behavior would generate something like this:
+
+    ```js
+    var require_esm_file = __commonJS((exports) => {
+      __markAsModule(exports);
+      __export(exports, {
+        foo: () => foo
+      });
+      var foo = bar();
+    });
+    console.log(require_esm_file().foo);
+    ```
+
+    This behavior has been changed and esbuild now generates something like this instead:
+
+    ```js
+    var esm_file_exports = {};
+    __export(esm_file_exports, {
+      foo: () => foo
+    });
+    var foo;
+    var init_esm_file = __esm(() => {
+      foo = bar();
+    });
+    console.log((init_esm_file(), esm_file_exports).foo);
+    ```
+
+    The variables have been pulled out of the lazily-initialized closure and are accessible to the rest of the module's scope. Some benefits of this approach:
+
+    * If another file does `import {foo} from "./esm-file.js"`, it will just reference `foo` directly and will not pay the performance penalty or code size overhead of the dynamic property accesses that come with CommonJS-style exports. So this improves performance and reduces code size in some cases.
+
+    * This fixes a long-standing bug ([#706](https://github.com/evanw/esbuild/issues/706)) where entry point exports could be broken if the entry point is a target of a `require()` call and the output format was ESM. This happened because previously calling `require()` on an entry point converted it to CommonJS, which then meant it only had a single `default` export, and the exported variables were inside the CommonJS closure and inaccessible to an ESM-style `export {}` clause. Now calling `require()` on an entry point only causes it to be lazily-initialized but all exports are still in the module scope and can still be exported using a normal `export {}` clause.
+
+    * Now that this has been changed, `import()` of a module with top-level await ([#253](https://github.com/evanw/esbuild/issues/253)) is now allowed when code splitting is disabled. Previously this didn't work because `import()` with code splitting disabled was implemented by converting the module to CommonJS and using `Promise.resolve().then(() => require())`, but converting a module with top-level await to CommonJS is impossible because the CommonJS call signature must be synchronous. Now that this implemented using lazy initialization instead of CommonJS conversion, the closure wrapping the ESM file can now be `async` and the `import()` expression can be replaced by a call to the lazy initializer.
+
+    * Adding the ability for ESM files to be lazily-initialized is an important step toward additional future code splitting improvements including: manual chunk names ([#207](https://github.com/evanw/esbuild/issues/207)), correct import evaluation order ([#399](https://github.com/evanw/esbuild/issues/399)), and correct top-level await evaluation order ([#253](https://github.com/evanw/esbuild/issues/253)). These features all need to make use of deferred evaluation of ESM code.
+
+    In addition, calling `require()` on an ESM file now recursively wraps all transitive dependencies of that file instead of just wrapping that ESM file itself. This is an increase in the size of the generated code, but it is important for correctness ([#667](https://github.com/evanw/esbuild/issues/667)). Calling `require()` on a module means its evaluation order is determined at run-time, which means the evaluation order of all dependencies must also be determined at run-time. If you don't want the increase in code size, you should use an `import` statement instead of a `require()` call.
+
 ## Unreleased
 
 * Warn about mutation of private methods ([#1067](https://github.com/evanw/esbuild/pull/1067))

internal/bundler/bundler.go

@@ -91,6 +91,8 @@ type reprJS struct {
 	// A JavaScript stub is automatically generated for a CSS file when it's
 	// imported from a JavaScript file.
 	cssSourceIndex ast.Index32
+
+	didWrapDependencies bool
 }
 
 func (repr *reprJS) importRecords() *[]ast.ImportRecord {
@@ -1535,8 +1537,8 @@ func (s *scanner) processScannedFiles() []file {
 	// can't be constructed earlier because we generate new parse results for
 	// JavaScript stub files for CSS imports above.
 	files := make([]file, len(s.results))
-	for sourceIndex, result := range s.results {
-		if result.ok {
+	for sourceIndex := range s.results {
+		if result := &s.results[sourceIndex]; result.ok {
 			s.validateTLA(uint32(sourceIndex))
 			files[sourceIndex] = result.file
 		}
@@ -1555,9 +1557,7 @@ func (s *scanner) validateTLA(sourceIndex uint32) tlaCheck {
 			}
 
 			for importRecordIndex, record := range repr.ast.ImportRecords {
-				if record.SourceIndex.IsValid() &&
-					(record.Kind == ast.ImportRequire || record.Kind == ast.ImportStmt ||
-						(record.Kind == ast.ImportDynamic && !s.options.CodeSplitting)) {
+				if record.SourceIndex.IsValid() && (record.Kind == ast.ImportRequire || record.Kind == ast.ImportStmt) {
 					parent := s.validateTLA(record.SourceIndex.GetIndex())
 					if !parent.parent.IsValid() {
 						continue
@@ -1571,9 +1571,8 @@ func (s *scanner) validateTLA(sourceIndex uint32) tlaCheck {
 						continue
 					}
 
-					// Require of a top-level await chain is forbidden. Dynamic import of
-					// a top-level await chain is also forbidden if code splitting is off.
-					if record.Kind == ast.ImportRequire || (record.Kind == ast.ImportDynamic && !s.options.CodeSplitting) {
+					// Require of a top-level await chain is forbidden
+					if record.Kind == ast.ImportRequire {
 						var notes []logger.MsgData
 						var tlaPrettyPath string
 						otherSourceIndex := record.SourceIndex.GetIndex()
@@ -1604,27 +1603,27 @@ func (s *scanner) validateTLA(sourceIndex uint32) tlaCheck {
 						}
 
 						var text string
-						what := "require call"
-						why := ""
 						importedPrettyPath := s.results[record.SourceIndex.GetIndex()].file.source.PrettyPath
 
-						if record.Kind == ast.ImportDynamic {
-							what = "dynamic import"
-							why = " (enable code splitting to allow this)"
-						}
-
 						if importedPrettyPath == tlaPrettyPath {
-							text = fmt.Sprintf("This %s is not allowed because the imported file %q contains a top-level await%s",
-								what, importedPrettyPath, why)
+							text = fmt.Sprintf("This require call is not allowed because the imported file %q contains a top-level await",
+								importedPrettyPath)
 						} else {
-							text = fmt.Sprintf("This %s is not allowed because the transitive dependency %q contains a top-level await%s",
-								what, tlaPrettyPath, why)
+							text = fmt.Sprintf("This require call is not allowed because the transitive dependency %q contains a top-level await",
+								tlaPrettyPath)
 						}
 
 						s.log.AddRangeErrorWithNotes(&result.file.source, record.Range, text, notes)
 					}
 				}
 			}
+
+			// Make sure that if we wrap this module in a closure, the closure is also
+			// async. This happens when you call "import()" on this module and code
+			// splitting is off.
+			if result.tlaCheck.parent.IsValid() {
+				repr.meta.isAsyncOrHasAsyncDependency = true
+			}
 		}
 	}
 

internal/bundler/bundler_dce_test.go

@@ -1395,3 +1395,29 @@ func TestTreeShakingNoBundleIIFE(t *testing.T) {
 		},
 	})
 }
+
+func TestTreeShakingInESMWrapper(t *testing.T) {
+	dce_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/entry.js": `
+				import {keep1} from './lib'
+				console.log(keep1(), require('./cjs'))
+			`,
+			"/cjs.js": `
+				import {keep2} from './lib'
+				export default keep2()
+			`,
+			"/lib.js": `
+				export let keep1 = () => 'keep1'
+				export let keep2 = () => 'keep2'
+				export let REMOVE = () => 'REMOVE'
+			`,
+		},
+		entryPaths: []string{"/entry.js"},
+		options: config.Options{
+			Mode:          config.ModeBundle,
+			OutputFormat:  config.FormatESModule,
+			AbsOutputFile: "/out.js",
+		},
+	})
+}

internal/bundler/bundler_default_test.go

@@ -1868,7 +1868,7 @@ func TestThisWithES6Syntax(t *testing.T) {
 				import './es6-ns-export-namespace'
 				import './es6-ns-export-class'
 				import './es6-ns-export-abstract-class'
-				`,
+			`,
 			"/dummy.js": `export const dummy = 123`,
 			"/cjs.js":   `console.log(this)`,
 
@@ -3256,7 +3256,7 @@ entry.js: note: The top-level await in "entry.js" is here
 	})
 }
 
-func TestTopLevelAwaitForbiddenImportWithoutSplitting(t *testing.T) {
+func TestTopLevelAwaitAllowedImportWithoutSplitting(t *testing.T) {
 	default_suite.expectBundled(t, bundled{
 		files: map[string]string{
 			"/entry.js": `
@@ -3282,18 +3282,6 @@ func TestTopLevelAwaitForbiddenImportWithoutSplitting(t *testing.T) {
 			OutputFormat:  config.FormatESModule,
 			AbsOutputFile: "/out.js",
 		},
-		expectedScanLog: `entry.js: error: This dynamic import is not allowed because the transitive dependency "c.js" contains a top-level await (enable code splitting to allow this)
-a.js: note: The file "a.js" imports the file "b.js" here
-b.js: note: The file "b.js" imports the file "c.js" here
-c.js: note: The top-level await in "c.js" is here
-entry.js: error: This dynamic import is not allowed because the transitive dependency "c.js" contains a top-level await (enable code splitting to allow this)
-b.js: note: The file "b.js" imports the file "c.js" here
-c.js: note: The top-level await in "c.js" is here
-entry.js: error: This dynamic import is not allowed because the imported file "c.js" contains a top-level await (enable code splitting to allow this)
-c.js: note: The top-level await in "c.js" is here
-entry.js: error: This dynamic import is not allowed because the imported file "entry.js" contains a top-level await (enable code splitting to allow this)
-entry.js: note: The top-level await in "entry.js" is here
-`,
 	})
 }