fix serve with outfile (#707)

CHANGELOG.md

@@ -16,6 +16,10 @@
 
     The previous change to hide the automatically-generated N-API native node extensions from Yarn 2 writes these `*.node` files to the system's temporary directory. A new one was being created on each run which is wasteful even though they are only a few kilobytes in size. With this release `*.node` files will now be reused if they are already present in the system's temporary directory, so a new one is no longer created on each run. This fix was contributed by [@kzc](https://github.com/kzc).
 
+* Fix the serve API with `outfile` ([#707](https://github.com/evanw/esbuild/issues/707))
+
+    This release fixes a bug where the serve API did not work with the `outfile` setting. Using this setting with the serve API should now work fine.
+
 ## 0.8.36
 
 * Fix an issue with writing large files to stdout using the WebAssembly executable

pkg/api/api.go

@@ -285,7 +285,7 @@ type OutputFile struct {
 }
 
 func Build(options BuildOptions) BuildResult {
-	return buildImpl(options)
+	return buildImpl(options).result
 }
 
 ////////////////////////////////////////////////////////////////////////////////

pkg/api/api_impl.go

@@ -3,7 +3,6 @@ package api
 import (
 	"fmt"
 	"io/ioutil"
-	"math/rand"
 	"mime"
 	"net"
 	"net/http"
@@ -510,7 +509,12 @@ func convertMessagesToInternal(msgs []logger.Msg, kind logger.MsgKind, messages
 ////////////////////////////////////////////////////////////////////////////////
 // Build API
 
-func buildImpl(buildOpts BuildOptions) BuildResult {
+type internalBuildResult struct {
+	result  BuildResult
+	options config.Options
+}
+
+func buildImpl(buildOpts BuildOptions) internalBuildResult {
 	logOptions := logger.OutputOptions{
 		IncludeSource: true,
 		ErrorLimit:    buildOpts.ErrorLimit,
@@ -531,7 +535,7 @@ func rebuildImpl(
 	plugins []config.Plugin,
 	logOptions logger.OutputOptions,
 	log logger.Log,
-) BuildResult {
+) internalBuildResult {
 	// Convert and validate the buildOpts
 	realFS := fs.RealFS()
 	jsFeatures, cssFeatures := validateFeatures(log, buildOpts.Target, buildOpts.Engines)
@@ -726,17 +730,21 @@ func rebuildImpl(
 	var rebuild func() BuildResult
 	if buildOpts.Incremental {
 		rebuild = func() BuildResult {
-			return rebuildImpl(buildOpts, caches, plugins, logOptions, logger.NewStderrLog(logOptions))
+			return rebuildImpl(buildOpts, caches, plugins, logOptions, logger.NewStderrLog(logOptions)).result
 		}
 	}
 
 	msgs := log.Done()
-	return BuildResult{
+	result := BuildResult{
 		Errors:      convertMessagesToPublic(logger.Error, msgs),
 		Warnings:    convertMessagesToPublic(logger.Warning, msgs),
 		OutputFiles: outputFiles,
 		Rebuild:     rebuild,
 	}
+	return internalBuildResult{
+		result:  result,
+		options: options,
+	}
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -991,7 +999,7 @@ func loadPlugins(fs fs.FS, log logger.Log, plugins []Plugin) (results []config.P
 
 type apiHandler struct {
 	mutex        sync.Mutex
-	outdir       string
+	options      *config.Options
 	onRequest    func(ServeOnRequestArgs)
 	rebuild      func() BuildResult
 	currentBuild *runningBuild
@@ -1107,7 +1115,7 @@ func (h *apiHandler) ServeHTTP(res http.ResponseWriter, req *http.Request) {
 
 		// Check the output files for a match
 		for _, file := range result.OutputFiles {
-			if relPath, ok := h.fs.Rel(h.outdir, file.Path); ok {
+			if relPath, ok := h.fs.Rel(h.options.AbsOutputDir, file.Path); ok {
 				relPath = strings.ReplaceAll(relPath, "\\", "/")
 
 				// An exact match
@@ -1169,14 +1177,17 @@ func (h *apiHandler) ServeHTTP(res http.ResponseWriter, req *http.Request) {
 }
 
 func serveImpl(serveOptions ServeOptions, buildOptions BuildOptions) (ServeResult, error) {
-	// The output directory isn't actually ever written to. It just needs to be
-	// very unlikely to be used as a source file so it doesn't collide.
 	realFS := fs.RealFS()
-	outdir := realFS.Join(os.TempDir(), strconv.FormatInt(rand.NewSource(time.Now().Unix()).Int63(), 36))
 	buildOptions.Incremental = true
 	buildOptions.Write = false
-	buildOptions.Outfile = ""
-	buildOptions.Outdir = outdir
+
+	// If there is no output directory, set the output directory to something so
+	// the build doesn't try to write to stdout. Make sure not to set this to a
+	// path that may contain the user's files in it since we don't want to get
+	// errors about overwriting input files.
+	if buildOptions.Outdir == "" && buildOptions.Outfile == "" {
+		buildOptions.Outdir = realFS.Join(realFS.Cwd(), "...")
+	}
 
 	// Pick the port
 	var listener net.Listener
@@ -1215,11 +1226,17 @@ func serveImpl(serveOptions ServeOptions, buildOptions BuildOptions) (ServeResul
 	}
 
 	// The first build will just build normally
-	handler := &apiHandler{
-		outdir:    outdir,
+	var handler *apiHandler
+	handler = &apiHandler{
 		onRequest: serveOptions.OnRequest,
-		rebuild:   func() BuildResult { return Build(buildOptions) },
-		fs:        realFS,
+		rebuild: func() BuildResult {
+			build := buildImpl(buildOptions)
+			if handler.options == nil {
+				handler.options = &build.options
+			}
+			return build.result
+		},
+		fs: realFS,
 	}
 
 	// Start the server

scripts/js-api-tests.js

@@ -1519,6 +1519,18 @@ console.log("success");
   },
 }
 
+function fetch(host, port, path) {
+  return new Promise((resolve, reject) => {
+    http.get({ host, port, path }, res => {
+      if (res.statusCode < 200 || res.statusCode > 299)
+        return reject(new Error(`${res.statusCode} when fetching ${path}`))
+      const chunks = []
+      res.on('data', chunk => chunks.push(chunk))
+      res.on('end', () => resolve(Buffer.concat(chunks)))
+    }).on('error', reject)
+  })
+}
+
 let serveTests = {
   async basic({ esbuild, service, testDir }) {
     for (const toTest of [esbuild, service]) {
@@ -1530,21 +1542,14 @@ let serveTests = {
         onRequest = resolve;
       });
 
-      const result = await toTest.serve({ onRequest }, { entryPoints: [input], format: 'esm' })
+      const result = await toTest.serve({ onRequest }, {
+        entryPoints: [input],
+        format: 'esm',
+      })
       assert.strictEqual(result.host, '127.0.0.1');
       assert.strictEqual(typeof result.port, 'number');
 
-      const buffer = await new Promise((resolve, reject) => {
-        http.get({
-          host: result.host,
-          port: result.port,
-          path: '/in.js',
-        }, res => {
-          const chunks = []
-          res.on('data', chunk => chunks.push(chunk))
-          res.on('end', () => resolve(Buffer.concat(chunks)))
-        }).on('error', reject)
-      })
+      const buffer = await fetch(result.host, result.port, '/in.js')
       assert.strictEqual(buffer.toString(), `console.log(123);\n`);
 
       let singleRequest = await singleRequestPromise;
@@ -1558,6 +1563,47 @@ let serveTests = {
       await result.wait;
     }
   },
+
+  async outfile({ esbuild, service, testDir }) {
+    for (const toTest of [esbuild, service]) {
+      const input = path.join(testDir, 'in.js')
+      await writeFileAsync(input, `console.log(123)`)
+
+      let onRequest;
+      let singleRequestPromise = new Promise(resolve => {
+        onRequest = resolve;
+      });
+
+      const result = await toTest.serve({ onRequest }, {
+        entryPoints: [input],
+        format: 'esm',
+        outfile: 'out.js',
+      })
+      assert.strictEqual(result.host, '127.0.0.1');
+      assert.strictEqual(typeof result.port, 'number');
+
+      const buffer = await fetch(result.host, result.port, '/out.js')
+      assert.strictEqual(buffer.toString(), `console.log(123);\n`);
+
+      try {
+        await fetch(result.host, result.port, '/in.js')
+        throw new Error('Expected a 404 error for "/in.js"')
+      } catch (err) {
+        if (err.message !== '404 when fetching /in.js')
+          throw err
+      }
+
+      let singleRequest = await singleRequestPromise;
+      assert.strictEqual(singleRequest.method, 'GET');
+      assert.strictEqual(singleRequest.path, '/out.js');
+      assert.strictEqual(singleRequest.status, 200);
+      assert.strictEqual(typeof singleRequest.remoteAddress, 'string');
+      assert.strictEqual(typeof singleRequest.timeInMS, 'number');
+
+      result.stop();
+      await result.wait;
+    }
+  },
 }
 
 async function futureSyntax(service, js, targetBelow, targetAbove) {