From 9abb31dd13a4b81d0d4f66a683bcab8eabb7b691 Mon Sep 17 00:00:00 2001
From: Evan Wallace <evan.exe@gmail.com>
Date: Wed, 16 Feb 2022 18:59:43 +0000
Subject: [PATCH] fix #1967: onResolve + onLoad now block on onStart

---
 CHANGELOG.md                |  4 ++++
 internal/bundler/bundler.go | 38 ++++++++++++++++++++++++++++++++++++-
 scripts/plugin-tests.js     | 30 +++++++++++++++++++++++++++++
 3 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9a62a473f2b..ffe6128135b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -33,6 +33,10 @@
     a,b{color:red}/* @preserve */
     ```
 
+* Block `onResolve` and `onLoad` until `onStart` ends ([#1967](https://github.com/evanw/esbuild/issues/1967))
+
+    This release changes the semantics of the `onStart` callback. All `onStart` callbacks from all plugins are run concurrently so that a slow plugin doesn't hold up the entire build. That's still the case. However, previously the only thing waiting for the `onStart` callbacks to finish was the end of the build. This meant that `onResolve` and/or `onLoad` callbacks could sometimes run before `onStart` had finished. This was by design but violated user expectations. With this release, all `onStart` callbacks must finish before any `onResolve` and/or `onLoad` callbacks are run.
+
 * Update to Go 1.17.7
 
     The version of the Go compiler used to compile esbuild has been upgraded from Go 1.17.6 to Go 1.17.7, which contains a few [compiler and security bug fixes](https://github.com/golang/go/issues?q=milestone%3AGo1.17.7+label%3ACherryPickApproved).
diff --git a/internal/bundler/bundler.go b/internal/bundler/bundler.go
index 72b08efa885..b7c1cc80302 100644
--- a/internal/bundler/bundler.go
+++ b/internal/bundler/bundler.go
@@ -1077,12 +1077,48 @@ func ScanBundle(
 		}
 	}()
 
+	// Wait for all "onStart" plugins here before continuing. People sometimes run
+	// setup code in "onStart" that "onLoad" expects to be able to use without
+	// "onLoad" needing to block on the completion of their "onStart" callback.
+	//
+	// We want to enable this:
+	//
+	//   let plugin = {
+	//     name: 'example',
+	//     setup(build) {
+	//       let started = false
+	//       build.onStart(() => started = true)
+	//       build.onLoad({ filter: /.*/ }, () => {
+	//         assert(started === true)
+	//       })
+	//     },
+	//   }
+	//
+	// without people having to write something like this:
+	//
+	//   let plugin = {
+	//     name: 'example',
+	//     setup(build) {
+	//       let started = {}
+	//       started.promise = new Promise(resolve => {
+	//         started.resolve = resolve
+	//       })
+	//       build.onStart(() => {
+	//         started.resolve(true)
+	//       })
+	//       build.onLoad({ filter: /.*/ }, async () => {
+	//         assert(await started.promise === true)
+	//       })
+	//     },
+	//   }
+	//
+	onStartWaitGroup.Wait()
+
 	s.preprocessInjectedFiles()
 	entryPointMeta := s.addEntryPoints(entryPoints)
 	s.scanAllDependencies()
 	files := s.processScannedFiles()
 
-	onStartWaitGroup.Wait()
 	return Bundle{
 		fs:              fs,
 		res:             res,
diff --git a/scripts/plugin-tests.js b/scripts/plugin-tests.js
index 5709b702d55..c84ddc30680 100644
--- a/scripts/plugin-tests.js
+++ b/scripts/plugin-tests.js
@@ -2650,6 +2650,36 @@ let syncTests = {
     result.rebuild.dispose()
   },
 
+  async onStartCallbackWithDelay({ esbuild }) {
+    await esbuild.build({
+      entryPoints: ['foo'],
+      write: false,
+      logLevel: 'silent',
+      plugins: [
+        {
+          name: 'some-plugin',
+          setup(build) {
+            let isStarted = false
+            build.onStart(async () => {
+              await new Promise(r => setTimeout(r, 1000))
+              isStarted = true
+            })
+
+            // Verify that "onStart" is finished before "onResolve" and "onLoad" run
+            build.onResolve({ filter: /foo/ }, () => {
+              assert.strictEqual(isStarted, true)
+              return { path: 'foo', namespace: 'foo' }
+            })
+            build.onLoad({ filter: /foo/ }, () => {
+              assert.strictEqual(isStarted, true)
+              return { contents: '' }
+            })
+          },
+        },
+      ],
+    })
+  },
+
   async onEndCallback({ esbuild, testDir }) {
     const input = path.join(testDir, 'in.js')
     await writeFileAsync(input, ``)