From 0ac0dbad1ccd85fa83ca3f51f7121a0f9f2ac654 Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Sat, 30 Jan 2021 02:44:20 -0800 Subject: [PATCH] fix serve with outfile (#707) --- CHANGELOG.md | 4 +++ pkg/api/api.go | 2 +- pkg/api/api_impl.go | 49 +++++++++++++++++++---------- scripts/js-api-tests.js | 70 ++++++++++++++++++++++++++++++++++------- 4 files changed, 96 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ab25165026d..770aeb6fe9c 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/pkg/api/api.go b/pkg/api/api.go index 3ec569698b2..6785ab6be9b 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -285,7 +285,7 @@ type OutputFile struct { } func Build(options BuildOptions) BuildResult { - return buildImpl(options) + return buildImpl(options).result } //////////////////////////////////////////////////////////////////////////////// diff --git a/pkg/api/api_impl.go b/pkg/api/api_impl.go index 39435b04697..de2a7e25d7b 100644 --- a/pkg/api/api_impl.go +++ b/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 diff --git a/scripts/js-api-tests.js b/scripts/js-api-tests.js index a64e2a9d8be..578c6cf03eb 100644 --- a/scripts/js-api-tests.js +++ b/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) {