Skip to content

Commit

Permalink
Merge branch 'main' into aix
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw authored Dec 18, 2023
2 parents f3214ad + f38cbe6 commit bc8a6c2
Show file tree
Hide file tree
Showing 14 changed files with 162 additions and 35 deletions.
24 changes: 24 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,24 @@

## Unreleased

* Fix a panic when transforming optional chaining with `define` ([#3551](https://github.com/evanw/esbuild/issues/3551), [#3554](https://github.com/evanw/esbuild/pull/3554))

This release fixes a case where esbuild could crash with a panic, which was triggered by using `define` to replace an expression containing an optional chain. Here is an example:

```js
// Original code
console.log(process?.env.SHELL)

// Old output (with --define:process.env={})
/* panic: Internal error (while parsing "<stdin>") */

// New output (with --define:process.env={})
var define_process_env_default = {};
console.log(define_process_env_default.SHELL);
```
This fix was contributed by [@hi-ogawa](https://github.com/hi-ogawa).
* Work around a bug in node's CommonJS export name detector ([#3544](https://github.com/evanw/esbuild/issues/3544))
The export names of a CommonJS module are dynamically-determined at run time because CommonJS exports are properties on a mutable object. But the export names of an ES module are statically-determined at module instantiation time by using `import` and `export` syntax and cannot be changed at run time.
Expand Down Expand Up @@ -71,6 +89,12 @@
new Foo();
```
* Terminate the Go GC when esbuild's `stop()` API is called ([#3552](https://github.com/evanw/esbuild/issues/3552))
If you use esbuild with WebAssembly and pass the `worker: false` flag to `esbuild.initialize()`, then esbuild will run the WebAssembly module on the main thread. If you do this within a Deno test and that test calls `esbuild.stop()` to clean up esbuild's resources, Deno may complain that a `setTimeout()` call lasted past the end of the test. This happens when the Go is in the middle of a garbage collection pass and has scheduled additional ongoing garbage collection work. Normally calling `esbuild.stop()` will terminate the web worker that the WebAssembly module runs in, which will terminate the Go GC, but that doesn't happen if you disable the web worker with `worker: false`.
With this release, esbuild will now attempt to terminate the Go GC in this edge case by calling `clearTimeout()` on these pending timeouts.
* Publish builds for IBM AIX PowerPC 64-bit ([#3549](https://github.com/evanw/esbuild/issues/3549))
This release publishes a binary executable to npm for IBM AIX PowerPC 64-bit, which means that in theory esbuild can now be installed in that environment with `npm install esbuild`. This hasn't actually been tested yet. If you have access to such a system, it would be helpful to confirm whether or not doing this actually works.
Expand Down
5 changes: 1 addition & 4 deletions internal/bundler/bundler.go
Original file line number Diff line number Diff line change
Expand Up @@ -682,10 +682,7 @@ func ResolveFailureErrorTextSuggestionNotes(
}

if platform != config.PlatformNode {
pkg := path
if strings.HasPrefix(pkg, "node:") {
pkg = pkg[5:]
}
pkg := strings.TrimPrefix(path, "node:")
if resolver.BuiltInNodeModules[pkg] {
var how string
switch logger.API {
Expand Down
58 changes: 58 additions & 0 deletions internal/bundler_tests/bundler_default_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5329,6 +5329,64 @@ func TestDefineOptionalChainLowered(t *testing.T) {
})
}

// See: https://github.com/evanw/esbuild/issues/3551
func TestDefineOptionalChainPanicIssue3551(t *testing.T) {
defines := config.ProcessDefines(map[string]config.DefineData{
"x": {
DefineExpr: &config.DefineExpr{
Constant: &js_ast.ENumber{Value: 1},
},
},
"a.b": {
DefineExpr: &config.DefineExpr{
Constant: &js_ast.ENumber{Value: 1},
},
},
})
default_suite.expectBundled(t, bundled{
files: map[string]string{
"/id-define.js": `
x?.y.z;
(x?.y).z;
x?.y["z"];
(x?.y)["z"];
x?.y();
(x?.y)();
x?.y.z();
(x?.y).z();
x?.y["z"]();
(x?.y)["z"]();
delete x?.y.z;
delete (x?.y).z;
delete x?.y["z"];
delete (x?.y)["z"];
`,
"/dot-define.js": `
a?.b.c;
(a?.b).c;
a?.b["c"];
(a?.b)["c"];
a?.b();
(a?.b)();
a?.b.c();
(a?.b).c();
a?.b["c"]();
(a?.b)["c"]();
delete a?.b.c;
delete (a?.b).c;
delete a?.b["c"];
delete (a?.b)["c"];
`,
},
entryPaths: []string{"/id-define.js", "/dot-define.js"},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputDir: "/out",
Defines: &defines,
},
})
}

// See: https://github.com/evanw/esbuild/issues/2407
func TestDefineInfiniteLoopIssue2407(t *testing.T) {
defines := config.ProcessDefines(map[string]config.DefineData{
Expand Down
36 changes: 36 additions & 0 deletions internal/bundler_tests/snapshots/snapshots_default.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1081,6 +1081,42 @@ console.log([
(_a = a[b]) == null ? void 0 : _a[c]
]);

================================================================================
TestDefineOptionalChainPanicIssue3551
---------- /out/id-define.js ----------
// id-define.js
1?.y.z;
(1?.y).z;
1?.y["z"];
(1?.y)["z"];
1?.y();
(1?.y)();
1?.y.z();
(1?.y).z();
1?.y["z"]();
(1?.y)["z"]();
delete 1?.y.z;
delete (1?.y).z;
delete 1?.y["z"];
delete (1?.y)["z"];

---------- /out/dot-define.js ----------
// dot-define.js
1 .c;
1 .c;
1["c"];
1["c"];
1();
1();
1 .c();
1 .c();
1["c"]();
1["c"]();
delete 1 .c;
delete 1 .c;
delete 1["c"];
delete 1["c"];

================================================================================
TestDefineThis
---------- /out.js ----------
Expand Down
8 changes: 4 additions & 4 deletions internal/css_parser/css_parser_selector.go
Original file line number Diff line number Diff line change
Expand Up @@ -854,8 +854,8 @@ func (p *parser) parseNthIndex() (css_ast.NthIndex, bool) {
if strings.HasPrefix(text0, "-") {
bNeg = true
text0 = text0[1:]
} else if strings.HasPrefix(text0, "+") {
text0 = text0[1:]
} else {
text0 = strings.TrimPrefix(text0, "+")
}
if b, ok := parseInteger(text0); ok {
if bNeg {
Expand Down Expand Up @@ -887,8 +887,8 @@ func (p *parser) parseNthIndex() (css_ast.NthIndex, bool) {
if strings.HasPrefix(text0, "-") {
aSign = negative
text0 = text0[1:]
} else if strings.HasPrefix(text0, "+") {
text0 = text0[1:]
} else {
text0 = strings.TrimPrefix(text0, "+")
}
}

Expand Down
5 changes: 1 addition & 4 deletions internal/fs/fs_zip.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,7 @@ func tryToReadZipArchive(zipPath string, archive *zipFile) {

// Build an index of all files in the archive
for _, file := range reader.File {
baseName := file.Name
if strings.HasSuffix(baseName, "/") {
baseName = baseName[:len(baseName)-1]
}
baseName := strings.TrimSuffix(file.Name, "/")
dirPath := ""
if slash := strings.LastIndexByte(baseName, '/'); slash != -1 {
dirPath = baseName[:slash]
Expand Down
11 changes: 7 additions & 4 deletions internal/js_parser/js_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -13462,7 +13462,8 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
}

// Lower optional chaining if we're the top of the chain
containsOptionalChain := e.OptionalChain != js_ast.OptionalChainNone
containsOptionalChain := e.OptionalChain == js_ast.OptionalChainStart ||
(e.OptionalChain == js_ast.OptionalChainContinue && out.childContainsOptionalChain)
if containsOptionalChain && !in.hasChainParent {
return p.lowerOptionalChain(expr, in, out)
}
Expand Down Expand Up @@ -13620,7 +13621,8 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
}

// Lower optional chaining if we're the top of the chain
containsOptionalChain := e.OptionalChain != js_ast.OptionalChainNone
containsOptionalChain := e.OptionalChain == js_ast.OptionalChainStart ||
(e.OptionalChain == js_ast.OptionalChainContinue && out.childContainsOptionalChain)
if containsOptionalChain && !in.hasChainParent {
return p.lowerOptionalChain(expr, in, out)
}
Expand Down Expand Up @@ -14718,7 +14720,8 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
}

// Lower optional chaining if we're the top of the chain
containsOptionalChain := e.OptionalChain != js_ast.OptionalChainNone
containsOptionalChain := e.OptionalChain == js_ast.OptionalChainStart ||
(e.OptionalChain == js_ast.OptionalChainContinue && out.childContainsOptionalChain)
if containsOptionalChain && !in.hasChainParent {
return p.lowerOptionalChain(expr, in, out)
}
Expand Down Expand Up @@ -17407,7 +17410,7 @@ func (p *parser) prepareForVisitPass() {
if jsxImportSource := p.lexer.JSXImportSourcePragmaComment; jsxImportSource.Text != "" {
if !p.options.jsx.AutomaticRuntime {
p.log.AddIDWithNotes(logger.MsgID_JS_UnsupportedJSXComment, logger.Warning, &p.tracker, jsxImportSource.Range,
fmt.Sprintf("The JSX import source cannot be set without also enabling React's \"automatic\" JSX transform"),
"The JSX import source cannot be set without also enabling React's \"automatic\" JSX transform",
[]logger.MsgData{{Text: "You can enable React's \"automatic\" JSX transform for this file by using a \"@jsxRuntime automatic\" comment."}})
} else {
p.options.jsx.ImportSource = jsxImportSource.Text
Expand Down
1 change: 0 additions & 1 deletion internal/resolver/package_json.go
Original file line number Diff line number Diff line change
Expand Up @@ -1376,7 +1376,6 @@ func (r resolverQuery) esmPackageTargetReverseResolve(
return true, keyWithoutTrailingStar + starData, target.firstToken
}
}
break
}

case pjObject:
Expand Down
12 changes: 10 additions & 2 deletions lib/deno/wasm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,13 @@ import type * as types from "../shared/types"
import * as common from "../shared/common"
import * as ourselves from "./wasm"

interface Go {
_scheduledTimeouts: Map<number, ReturnType<typeof setTimeout>>
}

declare const ESBUILD_VERSION: string
declare let WEB_WORKER_SOURCE_CODE: string
declare let WEB_WORKER_FUNCTION: (postMessage: (data: Uint8Array) => void) => (event: { data: Uint8Array | ArrayBuffer | WebAssembly.Module }) => void
declare let WEB_WORKER_FUNCTION: (postMessage: (data: Uint8Array) => void) => (event: { data: Uint8Array | ArrayBuffer | WebAssembly.Module }) => Go

export let version = ESBUILD_VERSION

Expand Down Expand Up @@ -91,10 +95,14 @@ const startRunningService = async (wasmURL: string | URL, wasmModule: WebAssembl
} else {
// Run esbuild on the main thread
let onmessage = WEB_WORKER_FUNCTION((data: Uint8Array) => worker.onmessage!({ data }))
let go: Go | undefined
worker = {
onmessage: null,
postMessage: data => setTimeout(() => onmessage({ data })),
postMessage: data => setTimeout(() => go = onmessage({ data })),
terminate() {
if (go)
for (let timeout of go._scheduledTimeouts.values())
clearTimeout(timeout)
},
}
}
Expand Down
12 changes: 10 additions & 2 deletions lib/npm/browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,13 @@ import type * as types from "../shared/types"
import * as common from "../shared/common"
import * as ourselves from "./browser"

interface Go {
_scheduledTimeouts: Map<number, ReturnType<typeof setTimeout>>
}

declare const ESBUILD_VERSION: string
declare let WEB_WORKER_SOURCE_CODE: string
declare let WEB_WORKER_FUNCTION: (postMessage: (data: Uint8Array) => void) => (event: { data: Uint8Array | ArrayBuffer | WebAssembly.Module }) => void
declare let WEB_WORKER_FUNCTION: (postMessage: (data: Uint8Array) => void) => (event: { data: Uint8Array | ArrayBuffer | WebAssembly.Module }) => Go

export let version = ESBUILD_VERSION

Expand Down Expand Up @@ -85,10 +89,14 @@ const startRunningService = async (wasmURL: string | URL, wasmModule: WebAssembl
} else {
// Run esbuild on the main thread
let onmessage = WEB_WORKER_FUNCTION((data: Uint8Array) => worker.onmessage!({ data }))
let go: Go | undefined
worker = {
onmessage: null,
postMessage: data => setTimeout(() => onmessage({ data })),
postMessage: data => setTimeout(() => go = onmessage({ data })),
terminate() {
if (go)
for (let timeout of go._scheduledTimeouts.values())
clearTimeout(timeout)
},
}
}
Expand Down
3 changes: 3 additions & 0 deletions lib/shared/worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ onmessage = ({ data: wasm }: { data: WebAssembly.Module | string }) => {
stdin.push(data)
if (resumeStdin) resumeStdin()
}
return go
}

fs.read = (
Expand Down Expand Up @@ -76,6 +77,8 @@ onmessage = ({ data: wasm }: { data: WebAssembly.Module | string }) => {
postMessage(error)
},
)

return go
}

async function tryToInstantiateModule(wasm: WebAssembly.Module | string, go: Go): Promise<WebAssembly.Instance> {
Expand Down
10 changes: 4 additions & 6 deletions pkg/api/api_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -540,13 +540,11 @@ func validateResolveExtensions(log logger.Log, order []string) []string {

func validateLoaders(log logger.Log, loaders map[string]Loader) map[string]config.Loader {
result := bundler.DefaultExtensionToLoaderMap()
if loaders != nil {
for ext, loader := range loaders {
if ext != "" && !isValidExtension(ext) {
log.AddError(nil, logger.Range{}, fmt.Sprintf("Invalid file extension: %q", ext))
}
result[ext] = validateLoader(loader)
for ext, loader := range loaders {
if ext != "" && !isValidExtension(ext) {
log.AddError(nil, logger.Range{}, fmt.Sprintf("Invalid file extension: %q", ext))
}
result[ext] = validateLoader(loader)
}
return result
}
Expand Down
4 changes: 0 additions & 4 deletions scripts/deno-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,6 @@ function test(name, backends, fn) {
promise.then(cancel, cancel)
promise.then(resolve, reject)
}),

// It's ok that the Go WebAssembly runtime uses "setTimeout"
sanitizeResources: false,
sanitizeOps: false,
})

for (const backend of backends) {
Expand Down
8 changes: 4 additions & 4 deletions scripts/esbuild.js
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ exports.buildWasmLib = async (esbuildPath) => {
].concat(minifyFlags), { cwd: repoDir }).toString().replace('WEB_WORKER_FUNCTION', wasmWorkerCodeUMD)
fs.writeFileSync(path.join(libDir, minify ? 'browser.min.js' : 'browser.js'), browserCJS)

// Generate "npm/esbuild-wasm/esm/browser.min.js"
// Generate "npm/esbuild-wasm/esm/browser.*"
const browserESM = childProcess.execFileSync(esbuildPath, [
path.join(repoDir, 'lib', 'npm', 'browser.ts'),
'--bundle',
Expand Down Expand Up @@ -222,7 +222,7 @@ exports.buildWasmLib = async (esbuildPath) => {
}

const buildDenoLib = async (esbuildPath) => {
// Generate "deno/esbuild/mod.js"
// Generate "deno/mod.js"
childProcess.execFileSync(esbuildPath, [
path.join(repoDir, 'lib', 'deno', 'mod.ts'),
'--bundle',
Expand All @@ -234,7 +234,7 @@ const buildDenoLib = async (esbuildPath) => {
'--banner:js=/// <reference types="./mod.d.ts" />',
], { cwd: repoDir })

// Generate "deno/esbuild/wasm.js"
// Generate "deno/wasm.js"
const GOROOT = childProcess.execFileSync('go', ['env', 'GOROOT']).toString().trim()
let wasm_exec_js = fs.readFileSync(path.join(GOROOT, 'misc', 'wasm', 'wasm_exec.js'), 'utf8')
const wasmWorkerCode = await generateWorkerCode({ esbuildPath, wasm_exec_js, minify: true, target: 'esnext' })
Expand All @@ -250,7 +250,7 @@ const buildDenoLib = async (esbuildPath) => {
], { cwd: repoDir }).toString().replace('WEB_WORKER_FUNCTION', wasmWorkerCode)
fs.writeFileSync(path.join(denoDir, 'wasm.js'), modWASM)

// Generate "deno/esbuild/mod.d.ts"
// Generate "deno/mod.d.ts"
const types_ts = fs.readFileSync(path.join(repoDir, 'lib', 'shared', 'types.ts'), 'utf8') +
`\n// Unlike node, Deno lacks the necessary APIs to clean up child processes` +
`\n// automatically. You must manually call stop() in Deno when you're done` +
Expand Down

0 comments on commit bc8a6c2

Please sign in to comment.