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

Two core-js dependencies #1421

Closed
dmnd opened this issue Oct 20, 2016 · 13 comments
Closed

Two core-js dependencies #1421

dmnd opened this issue Oct 20, 2016 · 13 comments

Comments

@dmnd
Copy link
Contributor

dmnd commented Oct 20, 2016

Relay depends on core-js twice:

  1. relay -> babel-runtime ^6.6.1 -> core-js ^2.1.0
  2. relay -> fbjs ^0.8.1 -> core-js ^1.0.0

These two versions aren't semver compatible, so it means a project that includes relay will have two copies of core-js in use. It'd be nice to collapse this dependency into a single version of core-js.

You can probably fix this by depending on fbjs 0.9.0-alpha.1 but maybe that's labelled alpha for a reason?

@wincent
Copy link
Contributor

wincent commented Jan 30, 2017

Well spotted! Thanks for bringing this up @dmnd.

Some dev dependencies (like react, react-dom) end up depending on a non-alpha version of fbjs. I know they're just dev dependencies, but I'm inclined to wait until the next major updates there as well and do it all in one fell swoop. What do you think?

@dmnd
Copy link
Contributor Author

dmnd commented Jan 30, 2017

I'm using Relay in production, so in the interest of asset sizes, I'd prefer to collapse the duplicate dependencies sooner where possible 😉

Who do we need to convince to release a proper version of fbjs?

@wincent
Copy link
Contributor

wincent commented Jan 30, 2017

First thing's first is probably opening an issue on the repo, but I see you just did that... I can also ping internally to ask why we have the alpha release at all anyway (it's not tagged in GitHub).

@yasserkaddour
Copy link
Contributor

I don't think the double dependency is much of an issue since fbjs only uses es6/set and es6/map for a total of ~9kb, whereas the latest version of babel-polyfill import all the core-js library for a total of ~89kb.
I think sticking to an earlier version of babel-polyfill would be better, I just tried successfully babel-es6-polyfill now core-js is ~25kb, I think knowing what is exactly needed in core-js would reduce the bundle size further.

Speaking of bundle size; I hope relay 2 will be sensible to it, and would allow modular import; ~214kb / ~75kb gziped is a bit too much. And Preact compatibility would be super preactjs/preact#411.

I am really exited about relay 2! Keep up the good work 👍 Thanks

@wincent
Copy link
Contributor

wincent commented Jan 31, 2017

Yep @yasserkaddour. I'd like to see more granular modular imports moving forward too. Let's make it happen.

@yasserkaddour
Copy link
Contributor

yasserkaddour commented Jan 31, 2017

🎉 @wincent good to hear. Can I know what babel-polyfill is used for exactly ? Thanks

@wincent
Copy link
Contributor

wincent commented Feb 1, 2017

@yasserkaddour, I believe it provides the regenerator runtime (which we're not currently using, but could use in the future for async support) and the so-called core-js package, which has a broad set of ES5/ES6 polyfills. At some point it would be desirable to make it possible to bring-your-own-polyfill, so that if users knew they only had to support modern environments they could just drop the polyfill altogether.

@mattecapu
Copy link

+1 for the size reduction
Relay takes currently about a quarter of my app bundle

@yasserkaddour
Copy link
Contributor

yasserkaddour commented Feb 1, 2017

Sorry, I wasn't clear; I wanted to know what subset of es5/es6 function is used by relay? and why use a polyfill instead of a transpiler?
Aren't we already bringing our own polyfill since it up to the user to add import 'babel-polyfill' in his app, and chose what version to install?

@wincent
Copy link
Contributor

wincent commented Feb 1, 2017

I wanted to know what subset of es5/es6 function is used by relay?

I could give you a semi-accurate answer off the top of my head, but your best bet is probably to have a skim through the source code and have a look.

@ephys
Copy link

ephys commented Jun 19, 2017

How soon do you think until we can provide our own polyfills? We have 3 differents versions of core-js in our bundle and it's wasting a lot of space.

Also one of those versions is causing relay to crash because Set.size is null:

console.log('+++++++ LOADING APP ++++++++');
console.log('native set', (new Set()).size);

const CoreJsSet = require('babel-runtime/core-js/set').default;
console.log('babel-runtime/core-js set', (new CoreJsSet()).size);

screen shot 2017-06-19 at 18 47 39

@sibelius
Copy link
Contributor

@ephys polyfills have been removed from relay

#1870

@ephys
Copy link

ephys commented Jun 19, 2017

@sibelius Great to hear! Thank you

@kassens kassens closed this as completed Aug 31, 2017
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

7 participants