Skip to content
This repository was archived by the owner on May 14, 2020. It is now read-only.

Remove babel runtime dependency #70

Closed
wants to merge 2 commits into from

Conversation

lgeiger
Copy link
Contributor

@lgeiger lgeiger commented Mar 4, 2017

This PR removes the usage of Symbol.iterator in order to remove babel-runtime while (hopefully 😄) still supporting IE11.

This reduces the package size from 2.3 MB down to only 300 kB.

The PR relies on the fact that iterable objects should have a .forEach method. At least that's my intuition, though I'm not an expert on this particular subject.

I added tests for the types mentioned in JSONNode.js though I couldn't come up with test cases for type Custom and Iterable.

@alexkuz
Copy link
Owner

alexkuz commented Mar 5, 2017

Thank you! I'm not sure about this PR though.

First, babel-runtime is used not just for Symbol.iterator, there's quite a few other things that are handled with babel. I don't know how well it is supported in modern browsers.

Second, what is that package we are talking about, exactly? I've updated example with production build, you can take a look:

cd examples && npm i && npm run stats

If you look at generated examples/dist/stats.json (I recommend webpack-bundle-analizer), you'll see that compiled runtime-related code weights less than 20kb. And the size of development build doesn't really matter, I believe.

@lgeiger
Copy link
Contributor Author

lgeiger commented Mar 5, 2017

First, babel-runtime is used not just for Symbol.iterator, there's quite a few other things that are handled with babel. I don't know how well it is supported in modern browsers.

I looked at #2 which adds babel-runtime as a dependency for supporting Symbol.iterator but since then it may be needed for other stuff as well.

We are using it in transforms for nteract which are used without webpack and required as a normal npm module. This means the size of the node_modules/ folder matters.

I don't know if there's an other solution for reducing node_modules/ size while retaining legacy browser support. So we may have to live with the increased package size.

@alexkuz
Copy link
Owner

alexkuz commented Mar 5, 2017

I see. Anyway, there should be code optimization at some point, if this project is meant to be used in production, so it shouldn't be a big problem for end user. I share your dislike for huge packages, but it's not that easy to fix in this case.

@lgeiger
Copy link
Contributor Author

lgeiger commented Mar 5, 2017

That's OK, I was hoping this is a quick fix to reduces the package size. I had in mind using it as well in an Atom package without code optimization.
Thanks for this package 👍.

I'll extract the tests into another PR and close this one.

@lgeiger lgeiger closed this Mar 5, 2017
@lgeiger lgeiger deleted the remove-babel-runtime branch March 5, 2017 11:07
@lgeiger lgeiger mentioned this pull request Mar 5, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants