evanw / esbuild

An extremely fast bundler for the web
https://esbuild.github.io/
MIT License
38.1k stars 1.15k forks source link

Bug: global this is optimized to void 0, which will lead to runtime errors. #1225

Closed cookiengineer closed 3 years ago

cookiengineer commented 3 years ago

In the source code, I'm using a way to detect a global variable that's checking against global, window and then fallsback to this. In the bundled code of esbuild, the this part is optimized to void 0 (which is undefined) and therefore will throw some errors in the fallback case of using the global scope as a definition scope.

Why is this important? "Sandboxing" in Browsers is the reason. I had to implement helper methods like isString(), isArray() etc in order to check against data types that are transferred across sandboxes (or iframes in the Web Browser case). In this scenario something like instanceof will fail as it's checking against a datatype that's defined as a unique Function template in the current sandbox.

So window.Array !== other_window.Array and in nodejs, the same applies when using separate Context instances in v8.

In order to fix this, I've built a little gluecode library that offers checks and exports of the same datatypes (along polyfills for Buffer etc). In my case however, this transpilation scenario seems to fail, as it would break in environments where only the globalThis is present (like it is the case in nodejs-compatible embedded runtimes like jerryscript, or when embedding libnode.so on an Android App).

How to reproduce:

Source Code:

export const Array = (typeof global !== 'undefined' ? global : (typeof window !== 'undefined' ? window : this)).Array;

if (typeof Array.isArray !== 'function') {

    Array.isArray = function(arr) {
        return Object.prototype.toString.call(arr) === '[object Array]';
    };

}

export const isArray = Array.isArray;

Transformed Outfile (target: es2020 and format: esm):

const Array = (typeof global !== "undefined" ? global : typeof window !== "undefined" ? window : void 0).Array;
if (typeof Array.isArray !== "function") {
  Array.isArray = function(arr) {
    return Object.prototype.toString.call(arr) === "[object Array]";
  };
}
const isArray = Array.isArray;
export {
  Array,
  isArray
};
evanw commented 3 years ago

I do not consider behavior you described to be a bug. The top-level value of this is undefined in ECMAScript modules, and the code you wrote is considered an ECMAScript module because of the export keyword. So replacing this with undefined is necessary to be faithful to ECMAScript module semantics. Although esbuild doesn't warn about this, other bundlers such as Rollup do warn about this (and will warn about your code above and also replace this with undefined).

I believe this is specifically because ECMAScript modules are always evaluated in strict mode (section 10.2.1 of the ES6 specification):

Module code is always strict mode code.

and strict mode functions do not coerce undefined to the global object (Annex C of the ES6 specification):

If this is evaluated within strict mode code, then the this value is not coerced to an object. A this value of null or undefined is not converted to the global object and primitive values are not converted to wrapper objects. The this value passed via a function call (including calls made using Function.prototype.apply and Function.prototype.call) do not coerce the passed this value to an object (9.2.1.2, 19.2.3.1, 19.2.3.3).

The new globalThis property is intended to be a portable way to reference the global object, and could be something to look into. Lack of support for globalThis in older browsers could be an issue but there is a shim for globalThis. You could also consider escaping strict mode by evaluating code in the global scope. For example, you can use the Function constructor:

let globalThisRef = new Function('return this')()

Edit: Also this is equal to exports in CommonJS modules. You can see this for yourself in node:

$ cat example.mjs 
console.log(this, this === global)

$ node example.mjs 
undefined false

$ cat example.cjs 
console.log(this, this === global, this === exports)

$ node example.cjs 
{} false true

Since esbuild emulates both CommonJS module semantics and ECMAScript module semantics, this is either undefined (ESM) or exports (CommonJS) but this is never equal to the global object when bundling is enabled. In other words you will need to find another solution regardless of whether you are using ESM or CommonJS.

cookiengineer commented 3 years ago

I was actually also reading up on this, it seems that most of the embedded runtimes (jerryscript and others) are kind of spec-violating in this matter; as they have support for import/export but don't have support for globalThis (yet) and seem to execute most things in the same scope.

PS: Thanks much for the detailed reply. I was assuming this fallback behaviour would be the same across all Browsers, as back then when I was verifying it, I was testing it in an outdated Safari 10ish version where module support was pretty new.

So I guess in my case it's better to use globalThis instead, and shim it beforehand so that it's guaranteed to be the same across all potential environments.