Skip to content

Commit

Permalink
fix #842: include known globals for the browser
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Feb 19, 2021
1 parent 95be32c commit 29500e6
Show file tree
Hide file tree
Showing 4 changed files with 749 additions and 4 deletions.
45 changes: 45 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,51 @@

In version 0.8.43, esbuild added support for node's [`NODE_PATH`](https://nodejs.org/api/modules.html#modules_loading_from_the_global_folders) environment variable which contains a list of global folders to use during path resolution. However, this causes a problem when esbuild is installed with [pnpm](https://pnpm.js.org/), an alternative JavaScript package manager. Specifically pnpm adds a bogus path to `NODE_PATH` that doesn't exist but that has a file as a parent directory. Previously this caused esbuild to fail with the error `not a directory`. Now with this release, esbuild will ignore this bogus path instead of giving an error.

* Add more names to the global no-side-effect list ([#842](https://github.com/evanw/esbuild/issues/842))

This release adds almost all known globals from the browser and node to the list of known globals. Membership in this list means accessing the global is assumed to have no side effects. That means tree shaking is allowed to remove unused references to these globals. For example, since `HTMLElement` is now in the known globals list, the following class will now be removed when unused:

```js
class MyElement extends HTMLElement {
}
```

In addition, membership in this list relaxes ordering constraints for the purposes of minification. It allows esbuild to reorder references to these globals past other expressions. For example, since `console.log` is now in the known globals list, the following simplification will now be performed during minification:

```js
// Original
export default (a) => {
if (a) console.log(b); else console.log(c)
}

// Minified (previous release)
export default (a) => {
a ? console.log(b) : console.log(c);
};

// Minified (this release)
export default (a) => {
console.log(a ? b : c);
};
```

This transformation is not generally safe because the `console.log` property access might evaluate code which could potentially change the value of `a`. This is only considered safe in this instance because `console.log` is now in the known globals list.

Note that membership in this list does not say anything about whether the function has side effects when called. It only says that the identifier has no side effects when referenced. So `console.log()` is still considered to have side effects even though `console.log` is now considered to be free of side effects.

The following globals are not on the list and are considered to have side effects:

* `scrollX`
* `scrollY`
* `innerWidth`
* `innerHeight`
* `pageXOffset`
* `pageYOffset`
* `localStorage`
* `sessionStorage`

Accessing layout-related properties can trigger a layout and accessing storage-related properties can throw an exception if certain privacy settings are enabled. Both of these behaviors are considered side effects.

## 0.8.48

* Fix some parsing edge cases ([#835](https://github.com/evanw/esbuild/issues/835))
Expand Down
2 changes: 2 additions & 0 deletions internal/bundler/bundler_default_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3842,6 +3842,7 @@ func TestConstWithLet(t *testing.T) {
"/entry.js": `
const a = 1; console.log(a)
if (true) { const b = 2; console.log(b) }
if (true) { const b = 2; unknownFn(b) }
for (const c = x;;) console.log(c)
for (const d in x) console.log(d)
for (const e of x) console.log(e)
Expand All @@ -3862,6 +3863,7 @@ func TestConstWithLetNoBundle(t *testing.T) {
"/entry.js": `
const a = 1; console.log(a)
if (true) { const b = 2; console.log(b) }
if (true) { const b = 2; unknownFn(b) }
for (const c = x;;) console.log(c)
for (const d in x) console.log(d)
for (const e of x) console.log(e)
Expand Down
7 changes: 4 additions & 3 deletions internal/bundler/snapshots/snapshots_default.txt
Original file line number Diff line number Diff line change
Expand Up @@ -286,9 +286,10 @@ TestConstWithLet
// entry.js
var a = 1;
console.log(a);
console.log(2);
{
let b = 2;
console.log(b);
unknownFn(b);
}
for (let c = x; ; )
console.log(c);
Expand All @@ -301,10 +302,10 @@ for (let e of x)
TestConstWithLetNoBundle
---------- /out.js ----------
const a = 1;
console.log(a);
console.log(a), console.log(2);
{
const b = 2;
console.log(b);
unknownFn(b);
}
for (const c = x; ; )
console.log(c);
Expand Down

0 comments on commit 29500e6

Please sign in to comment.