zloirock / core-js

Standard Library
MIT License
24.03k stars 1.62k forks source link

Invalid global object detected when bundling with esbuild and running on Duktape #1303

Closed bjoluc closed 6 months ago

bjoluc commented 7 months ago

I know my environment is fairly edgy and I'm happy to work around it locally, but since I spent quite some time tracking my issue down already, I thought it wouldn't hurt to at least share my results here.

I bundle some pieces of core-js in an esbuild-built project that targets Duktape. Up to core-js 3.30.1, everything was working fine, but the fix for #1231 broke the global object detection in Duktape – here's what happens in the build output:

"use strict";
var __getOwnPropNames = Object.getOwnPropertyNames;
var __commonJS = function(cb, mod) {
    return function __require() {
        return mod || (0, cb[__getOwnPropNames(cb)[0]])((mod = {
            exports: {}
        }).exports, mod), mod.exports;
    };
};

// node_modules/core-js/internals/global.js
var require_global = __commonJS({
    "node_modules/core-js/internals/global.js": function(exports, module2) {
        "use strict";
        var check = function check(it) {
            return it && it.Math === Math && it;
        };
        module2.exports = check(typeof globalThis == "object" && globalThis) || check(typeof window == "object" && window) || // eslint-disable-next-line no-restricted-globals -- safe
        check(typeof self == "object" && self) || check(typeof global == "object" && global) || // eslint-disable-next-line no-new-func -- fallback
        function() {
            return this;
        }() || exports || Function("return this")();
    }
});

console.log(typeof require_global().Math);

Running the above code in Node.js correctly prints object, while Duktape decides to go for undefined, i.e. the object exported by global.js is not the global one. Sadly, I'm not involved enough to judge if this is related to https://github.com/evanw/esbuild/issues/1147 or not.

The following two patches work for me, but I don't know whether they might break again in Figma (#1231):

- (function () { return this; })() || this || Function('return this')();
+ (function () { return this; })() || Function('return this')() || this;
+ check(typeof this == 'object' && this) ||
   // eslint-disable-next-line no-new-func -- fallback
- (function () { return this; })() || this || Function('return this')();
+ (function () { return this; })() || Function('return this')();

Lastly, thanks for your continued work on core-js @zloirock :heart: I hope you'll receive more of the support you deserve in the future :smiling_face_with_tear:

zloirock commented 6 months ago

Ok, I think that we could accept the second option 👍

zloirock commented 6 months ago

BTW Duktape still doesn't support WeakMap at all? Because it's critical for https://github.com/zloirock/core-js/issues/1301

bjoluc commented 6 months ago

You're right about WeakMap, sadly. It also looks like that's not going to happen in the foreseeable future. Personally, I'm totally fine with dropping Duktape support in v4 – it is awesome enough that v3 works with Duktape and at some point it feels wasted to keep supporting a rarely used and almost stale legacy engine (I wish it weren't, but so it seems).