Bringing back support for Firefox (and other non-V8 browsers)

[olemartinorg]

This exported 'undefined' if the user agent/version didn't match, and for some reason beyond my understanding that value became {default:undefined} on import. While you can compare a literal object to a number, you cannot compare this special {default:undefined} object to a number, as you'd get a TypeError complaining about it. As more and more modules have been comparing known version numbers to the V8 version in order to prevent V8 deoptimization lately, these have started failing in Firefox (at least). All code I could find using this version number compares it in a way where '0' will be correctly evaluated as 'not one of those recent v8 versions we're guarding against', so it should work fine.

This might fix #764, but that issue has been fairly light on specific details, so maybe only time will tell if this fixes it. As described in that issue, I have been able to reproduce my specific issue as far back as 3.3.5.

[zloirock]

I don't see how it could be related to #764.

I dropped ToLength detection from array methods feature detection since it could cause hanging of some old engines and I don't see any possible workaround which could work everywhere.

[olemartinorg]

Sadly, your changes does not seem to fix the issue. I added a simple reproduction repo for your convenience in olemartinorg/core-js-firefox-bug.

Please note, I recently also found out the code fails in Safari as well as Firefox. I assume all non-chromium browsers will have this code crashing on load. In Firefox, i get a Uncaught TypeError: can't convert V8_VERSION$1 to number and in Safari I get TypeError: No default value.

Let me know if my reproduction example does not crash on load for you. I really appreciate core-js and your efforts!

[zloirock]

@olemartinorg the fix still is not published on NPM.

[zloirock]

Moreover, V8_VERSION absolutely is not related to #764.

[olemartinorg]

I know it's not published to NPM. I tried doing a npm install --save file://./../core-js/packages/core-js/ to test out the latest master, but the same problem still occurs there.

As for #764, the problem description there is still unclear to me, but the fact is still that this V8_VERSION problem causes a crash for me on Firefox (and Safari), so I assumed it best to not open a new issue when #764 seemed untriaged. Do you want me to open a new issue (with a new pull request) for this V8_VERSION issue?

[zloirock]

#764 is about the hanging of the browser, your issue - about crashing in another place.

As I could see:

// engine-v8-version:
module.exports = version && +version; // -> undefined or integer
// es.array.concat
var V8_VERSION = require('../internals/engine-v8-version');
// ...
// The error in this line: `Uncaught TypeError: can't convert V8_VERSION$1 to number`
var IS_CONCAT_SPREADABLE_SUPPORT = V8_VERSION >= 51 || somethingElse();

In your bundle V8_VERSION is Object.assign(Object.create(null), { default: undefined }), but should be undefined or an integer - they are properly converted to Number.

That meansn that the error in your bundling process - seems rollup does not properly handle default import. I'm not an expert in rollup and I can't say the reason of this problem, but it's definitely not a core-js bug.

[zloirock]

The bug in this rollup helper which does not properly handle undefined in default import:

  function __interopImport$D(ex) {
                          if (ex.__esModule) {
                              return ex;
                          }

                          if (ex.default !== undefined) {
                              return ex.default;
                          } 

                          return ex;
                      }

[olemartinorg]

Oh, wow, thanks! I'm still a bit green when it comes to this whole JS ecosystem. I'll take this up with the rollup team then. Thanks so much for the detective work!

EDIT: The issue is specific to rollup-plugin-commonjs-alternate, which I've been using to get HMR working with Nollup. I submitted a pull request for a fix here: PepsRyuu/rollup-plugin-commonjs-alternate#3