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

Break es6-shim into smaller, independent submodules #297

Open
appsforartists opened this issue Oct 20, 2014 · 23 comments
Open

Break es6-shim into smaller, independent submodules #297

appsforartists opened this issue Oct 20, 2014 · 23 comments

Comments

@appsforartists
Copy link

Libraries like prfun (incidentally recommended in the es6-shim README) augment the Promise prototype with additional methods. Promise.promisify is particularly nice.

I just tried incorporating the es6-shim into my Node 0.11.13 project to get access to the Array polyfills. Now, I'm getting Promise.promisify cannot be found errors. A quick skim through the shim source shows that es6-shim completely overwrites the global Promise object with its own polyfill when it isn't satisfied with the present implementation.

I could probably work around this by including es6-shim before prfun, but the library that includes prfun and the one that includes es6-shim have nothing to do with each other. It'd be a shame to introduce a needless dependency to work around this.

@appsforartists
Copy link
Author

The most sane solution could be modularizing the shim and using something like webpack to make a browser-distributable library. Then, someone who only needs "es6-shim/arrayPrototype" can require("es6-shim/arrayPrototype") without breaking code that has nothing to do with Arrays. People targeting the browser can still load the whole es6-shim distribution, you'd just use webpack to build it.

@ljharb
Copy link
Collaborator

ljharb commented Oct 20, 2014

We definitely shouldn't be recommending something that modifies globals in a non-spec-compliant way, that's insanity. I'll fix the README, thanks.

The problem here is prfun, not es6-shim - if you choose to use something that modifies globals in a non-spec-compliant way, all bets are off.

@ljharb ljharb closed this as completed in 671b5c4 Oct 20, 2014
@appsforartists
Copy link
Author

I understand the reluctance to recommend a project that muxes global objects, but that's not the issue here.

The issue is that ES6 Shim has a huge footprint and isn't distributed in a modular way. If you use one library that needs a small part of ES6 Shim (that's unrelated to promises), and another library that depends on something like prfun, you can end up with race conditions that break the app.

How do you feel about supporting require("es6-shim/array") or require("es6-shim/promise") as an alternative to the monolithic require("es6-shim")?

@appsforartists appsforartists changed the title Promise implementation clobbers custom methods Break es6-shim into smaller, independent submodules Oct 21, 2014
@ljharb
Copy link
Collaborator

ljharb commented Oct 21, 2014

#136 covers that somewhat.

That's not a bad approach, but it would mean we'd need to incorporate browserify in as a build step, and we'd need to build both minified and unminified versions of files each release.

@ljharb ljharb reopened this Oct 21, 2014
@cscott
Copy link
Collaborator

cscott commented Oct 21, 2014

As the author of prfun, I agree the problem if any is in prfun. But I don't
fully understand what is happening, since prfun will require('es6-shim')
before adding its methods. I suggest opening a bug against prfun and we
can investigate.

I don't think the mention of prfun in es6-shim is inappropriate -- unlike
other promise libraries, prfun is explicitly written to be loaded on top
of a es6-compliant promise implementation.

@appsforartists
Copy link
Author

@cscott I don't know if it's worth fixing. The problem is that you augment native Promises, then es6-shim declares them insufficient and replaces them with its polyfill. Then, when the code that relied on prfun runs, it errors, because prfun was effectively uninstalled when es6-shim replaced the native implementation.

As long as both es6-shim and prfun seek to modify the global Promise, conflicts are inevitable.

@cscott
Copy link
Collaborator

cscott commented Oct 21, 2014

@appsforartists please open a bug against prfun. prfun can use the same "is the promise insufficient" test that es6-shim uses. Alternatively, you can explicitly ask prfun to use es6-shim's promises, that's in the README.

@chrishyle
Copy link

👍

@paulmillr
Copy link
Owner

Why don't you just use individual shims?

@zloirock
Copy link

@paulmillr total size and compatibility? For example, Array.from individual shim not support iterators, which makes it almost useless.
P.S. core-js supports custom build and is split to commonjs submodules :)

@paulmillr
Copy link
Owner

Just sayin', I'm reluctant to the idea of ES6 shim consisting of 20 npm submodules. maybe something what @appsforartists mentioned

@ljharb
Copy link
Collaborator

ljharb commented Mar 10, 2015

Why does the number of submodules matter? The size of the built JS file used in a browser would be basically equivalent, and "more dependencies" is a good thing :-)

@paulmillr
Copy link
Owner

"more dependencies" is a good thing

not when you see npm making 1000 requests to the server on a slow connection. Ask jdalton for his terrible experience with breaking down lodash

@appsforartists
Copy link
Author

As a Webpack user, a monolithic library that touches a bunch of unrelated native prototypes is very unappealing. I didn't know about core-js when I opened this issue, but now that @zloirock has shared that link, I may go there the next time I need a polyfill because of its modularity.

(FWIW, I think @ljharb is on the right track - if you want to release a single file, just run Webpack over your repo. There's no reason to have all your code in a single script. That said, I'm unfamiliar with @jdalton's experience with lodash.)

@jdalton
Copy link
Contributor

jdalton commented Mar 10, 2015

❤️ modules.

@paulmillr
Copy link
Owner

@jdalton where are all github repos for lodash methods?

@appsforartists
Copy link
Author

@paulmillr - think files, not repos

@paulmillr
Copy link
Owner

hmm interesting

yeah, maybe that'll work. One repo, many files.

@monolithed
Copy link

+1

@ljharb ljharb mentioned this issue Mar 22, 2015
@gobwas
Copy link

gobwas commented Apr 7, 2015

+1

@monolithed
Copy link

It would be great to have the following bundles

Math
String
Number
Promise
RegExp
Array
Object
Reflect
Collection
Symbol

@ljharb
Copy link
Collaborator

ljharb commented May 3, 2015

If and when we do this, you'll be able to import a la Array, Array.prototype, Array.prototype.find - any granularity you wish.

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

No branches or pull requests

9 participants