ES5 bundle conflict with core-js v3

[robertknight]

The core-js project released a new version, v3.0.0, which changes the module layout. The generated fetch-mock es5 bundle in es5/lib.js requires core-js modules assuming core-js v2's layout. As a result, attempting to include fetch-mock in a browserify-generated bundle (eg. for use with a browser-based test runner such as Karma) fails.

Steps to reproduce

1. Save the following package.json into an empty directory:

{
  "name": "fetch-mock-corejs-v3-test",
  "version": "1.0.0",
  "devDependencies": {
    "browserify": "^16.2.3",
    "core-js": "^3.0.0",
    "fetch-mock": "^7.3.1"
  }
}

2. Run npm install
3. Run ./node_modules/.bin/browserify -r fetch-mock

Output

Error: Cannot find module 'core-js/modules/es6.typed.array-buffer' from '/private/tmp/fetch-mock-corejs-v3-test/node_modules/fetch-mock/es5'
    at /private/tmp/fetch-mock-corejs-v3-test/node_modules/browser-resolve/node_modules/resolve/lib/async.js:46:17
    at process (/private/tmp/fetch-mock-corejs-v3-test/node_modules/browser-resolve/node_modules/resolve/lib/async.js:173:43)
    at ondir (/private/tmp/fetch-mock-corejs-v3-test/node_modules/browser-resolve/node_modules/resolve/lib/async.js:188:17)
    at load (/private/tmp/fetch-mock-corejs-v3-test/node_modules/browser-resolve/node_modules/resolve/lib/async.js:69:43)
    at onex (/private/tmp/fetch-mock-corejs-v3-test/node_modules/browser-resolve/node_modules/resolve/lib/async.js:92:31)
    at /private/tmp/fetch-mock-corejs-v3-test/node_modules/browser-resolve/node_modules/resolve/lib/async.js:22:47
    at FSReqCallback.oncomplete (fs.js:158:21)

Notes

My suggested solution here would be to omit polyfills from fetch-mock itself and instead make it the responsibility of the package consumer to include polyfills, at least for ES standard library functionality. This avoids potential conflicts with other polyfills and in some cases will reduce bundle size.

Thoughts? I would be happy to submit a PR if the proposed change is agreeable.

[wheresrhys]

In the past I didn't prebundle polyfill, but there were so many issues raised relating to it which eventually meant I caved in and included it. In principle, I agree that it's the consumer's responsibility to polyfill, but the reality is that there are scenarios where that's not so easy.

The best solution - unless you can think of a better one - is to generate a new core-js 3 compatible bundle, save it as a new file name, and document that users in a core-js 3 environment will need to reference that exact file. In the next major version that could switch to being the default browser export, with a core-js 2 bundle being available for backwards compatibility

It's kinda tricky to implement as will require multiple builds, with different npm dependencies, within the publish step. If you're familiar with circleci a PR to implement it would be welcome, otherwise I will try to get round to it sometime.

[robertknight]

In principle, I agree that it's the consumer's responsibility to polyfill, but the reality is that there are scenarios where that's not so easy.

Do you have some examples of issues that have come up in the past?

Generating a bundle with core-js v3 would resolve my immediate problem but it would still result in a larger bundle than necessary, which slows down the test runner.

[gfortaine]

Hello,

+1 for this issue.

Our immediate workaround (using Karma) has been to load node_modules/fetch-mock/dist/es5/client-bundle.js from Files :

files: [
  "node_modules/fetch-mock/dist/es5/client-bundle.js" // http://www.wheresrhys.co.uk/fetch-mock/#usageinstallation
],

Très cordialement / Best Regards,

[thommyhh]

I would vote for a v8.0.0 which depends on core-js v3.x and so requires modules from the v3.x layout.

[thommyhh]

Another workaround that might help some people. I noticed, that the structure is different when I load the package from git instead of npm/yarn. Instead of require('fetch-mock') I need to use require('fetch-mock/src/client'). The big difference is, that there are no dependencies on any core-js modules when I include the package via git. So how do they get in there in the packaged version, when they obviously aren't needed?

[grug]

I suspect this will get more interesting, as Angular 8.0 came out today which means that people can't use fetch-mock in Angular 8.0+ as it stands.

@wheresrhys I'd love to help out on this issue. Would you like to work on this together? IIRC you're in London - happy to grab a beer with you to talk it over.

[wheresrhys]

Could do. I'm actually looking for more people to help maintain the library in general too. I'm away a lot over the next few weeks. If you're on twitter send me a DM and we can work something out

I'm planning a v8 release soon anyway to replace babel-polyfill with transpile-runtime. Could an opportune moment to bump core-js too, though a solution that is core-js version agnostic would be even better.

[grug]

Sweet - I've pinged you on Twitter :)

[travist]

We were able to solve this pretty easily by just forcing this library to have a dependency on core-js@2.

I know this seems counter-intuitive, but by making this library dependent on core-js@2, you are allowing dependent libraries to require core-js@3 and NPM will then install a "local" node_modules folder in this library and everything just starts to work. Here is the pull request that fixes it.

#432

[Dynalon]

I'd side with @robertknight that you shouldn't directly import from core-js but rather leave it to the consumer to establish ES5 compat. Babel comes with babel-polyfill which is essentially just core-js under the hood, and angular also uses core-js. FBs create-react-app also comes with babel-polyfill out of the box.

Additionally, you shouldn't probably hard-code polyfills for any ES5 stuff anymore for version 8.0, as there are hardly any modern browsers requiring polyfills at all. Given that fetch-mock is a prototyping/testing tool, support for IE11 etc. is hardly a requirement. And if you absolutely need support for older browsers, you'll probably have babel+core-js present anyways in your build stack or you can fall back to using fetch-mock 7.x

[jquense]

Please release a build without the polyfills!

[wheresrhys]

Have released with @travist's fix

Please give it a go

[grug]

@wheresrhys will this be published to NPM?

[CamBurris]

@wheresrhys Can we get a publish of the latest release to npm? It seems npm is behind by two versions at this point. It would be nice to have this fix published.

[bjankord]

I've tested a project that depends on core-js v3 and pulls in the fetch-mock v7.3.5 tarball and my webpack script ran successfully compiling the modules. Looking forward to seeing v7.3.5 released on npm.

[wheresrhys]

Damn - broken build. Sorry for the delay - missed the earlier message

Fixed in 7.3.6

[andreiho]

Somehow I'm stil experiencing this issue. I have core-js@3 installed in the root node_modules and core-js@2 installed locally in fetch-mock. I'm using TypeScript, not sure whether it makes a difference or not.

[gitclonedcush]

I'm still experiencing this issue as well. Same deal as @andreiho but not using TypeScript.

[jdelStrother]

Yeah, I'm still seeing the same problem as @andreiho & @cushind with 7.3.6. However... I only seem to get it when importing fetch-mock into a storybook build, rather than my regular webpack build. I'm still working on figuring out what the difference between the two setups is.

[andreiho]

@jdelStrother Same actually, only with Storybook.

[jdelStrother]

So Storybook uses corejs-upgrade-webpack-plugin which seems fairly broken - storybookjs/storybook#7445

In the meantime, I've hacked around it to remove that plugin in my storybook config. That looks something like this, in .storybook/webpack.config.js:

module.exports = function(options) {
  var config = options.config
  config.plugins = config.plugins.filter(
    p => String(p.resourceRegExp) !== "/core-js/"
  );
  return config

[wheresrhys]

v8.0.0-alpha.2 attempts to address this issue. Please leave a 👍 on the pull request #457 if your problem is solved, or alternatively leave a comment @i-like-robots @emyarod @BenjaminVanRyseghem @dancrumb @steveoh @joerodrig @andreiho @cushind @jdelStrother @mkay581 @sahariko @bjankord @CamBurris @kajyr @Dynalon @travist @thommyhh @gfortaine @robertknight @poorpaddy @iamnewton

[wheresrhys]

Closing in favour of discussion on the PR