Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Module polyfill override project polyfill #880

Closed
ZxBing0066 opened this issue Nov 3, 2020 · 6 comments
Closed

Module polyfill override project polyfill #880

ZxBing0066 opened this issue Nov 3, 2020 · 6 comments

Comments

@ZxBing0066
Copy link

In the following case, Promise.finally will be wrongly overwritten:

In project A I imported module B, the module bundled core-js with buildIn: usage, then it bundle polyfill with this in builded code:

require("core-js/modules/es.promise");

In project A, I want to bundle all polyfill into one file, so I use buildIn: entry to bundle a single polyfill.js, then in my project bundle, bundle html will be this:

<script src='polyfill.js'></script>
<script src='A.js'></script>

The polyfill.js load the right Promise, but in Edge 18, because of empty PromiseRejectionEvent, Promise will force override by PromiseConstructor:

// node_modules/core-js/modules/es.promise.js:53
var FORCED = isForced(PROMISE, function () {
  var GLOBAL_CORE_JS_PROMISE = inspectSource(PromiseConstructor) !== String(PromiseConstructor);
  if (!GLOBAL_CORE_JS_PROMISE) {
    if (V8_VERSION === 66) return true;
    // PromiseRejectionEvent is undefined, FORCED will be true
    if (!IS_NODE && typeof PromiseRejectionEvent != 'function') return true;
  }
  // ...
});

It's OK now, but when run into A.js, B module bundled core-js/modules/es.promise, without es.promise.finally, then it's broken, the var FORCED always to be true, the Promise will be override with the next one without finally.

So is there some good idea to avoid duplicated Promise polyfill load?
Such as add a tag to Promise polyfill, then in next polyfill running, check the tag to avoid duplicated overridden?

@zloirock
Copy link
Owner

zloirock commented Nov 5, 2020

Yes, this feature detection test case is problematic. I have no ideas about how to detect unhandled rejection tracking support synchronously with another way than check PromiseRejectionEvent. Do you have better ideas?

However, at first, your issue related to good practices of usage - you should load only one copy of the global core-js version.

Sure, good tooling could help to prevent problems like this. I have some ideas which could help to prevent problems like this in the next generation of tooling, but if it will be created - it will be created not soon.

@ZxBing0066
Copy link
Author

Yes, this feature detection test case is problematic. I have no ideas about how to detect unhandled rejection tracking support synchronously with another way than check PromiseRejectionEvent. Do you have better ideas?

However, at first, your issue related to good practices of usage - you should load only one copy of the global core-js version.

Sure, good tooling could help to prevent problems like this. I have some ideas which could help to prevent problems like this in the next generation of tooling, but if it will be created - it will be created not soon.

Yes, I know global single code-js is a good idea, but in some case, such as core-js loaded in some library or in some plugin, it's not uncontrolled by me.

Maybe we can add a custom property to the polyfilled PromiseConstructor such as __FROM_CORE_JS__, then in if(FORCED) check __FROM_CORE_JS__ and avoid to override the current.

// core-js/modules/es.promise.js:225

// check the current NativePromise should override
if (FORCED && !PromiseConstructor.__FROM_CORE_JS__) {
  PromiseConstructor = function Promise(executor) {
    // ...
  };
  // add the tag property
  PromiseConstructor.__FROM_CORE_JS__ = true;
}

@zloirock
Copy link
Owner

zloirock commented Nov 6, 2020

Maybe we can add a custom property to the polyfilled PromiseConstructor such as __FROM_CORE_JS__, then in if(FORCED) check __FROM_CORE_JS__ and avoid to override the current.

It's already here - see GLOBAL_CORE_JS_PROMISE check. Most likely one of your core-js versions is obsolete.

@ZxBing0066
Copy link
Author

Maybe we can add a custom property to the polyfilled PromiseConstructor such as __FROM_CORE_JS__, then in if(FORCED) check __FROM_CORE_JS__ and avoid to override the current.

It's already here - GLOBAL_CORE_JS_PROMISE. Most likely one of your core-js versions is obsolete.

No. My core-js version is 3.6.5.

The GLOBAL_CORE_JS_PROMISE has no effect in this case, both of inspectSource(PromiseConstructor) and String(PromiseConstructor) return the same code.

function Promise(executor) {
    anInstance(this, PromiseConstructor, PROMISE);
    aFunction(executor);
    Internal.call(this);
    var state = getInternalState(this);
    try {
      executor(bind(internalResolve, this, state), bind(internalReject, this, state));
    } catch (error) {
      internalReject(this, state, error);
    }
  };

@zloirock
Copy link
Owner

zloirock commented Nov 6, 2020

Interesting. String(PromiseConstructor) should be function Promise() { [native code] } because of Function#toString wrapper and it should work with multiple copies of core-js. However, I can confirm that it does not work with 2 copies of 3.6.5.

@ZxBing0066
Copy link
Author

It's fixed in 3.7.0, thank you.

😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants