paulmillr / es6-shim

ECMAScript 6 compatibility shims for legacy JS engines
http://paulmillr.com
MIT License
3.11k stars 389 forks source link

Break es6-shim into smaller, independent submodules #297

Open appsforartists opened 9 years ago

appsforartists commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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.

appsforartists commented 9 years ago

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")?

ljharb commented 9 years ago

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.

cscott commented 9 years ago

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 commented 9 years ago

@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 commented 9 years ago

@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 commented 9 years ago

:+1:

paulmillr commented 9 years ago

Why don't you just use individual shims?

zloirock commented 9 years ago

@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 commented 9 years ago

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

ljharb commented 9 years ago

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 commented 9 years ago

"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 commented 9 years ago

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 commented 9 years ago

:heart: modules.

paulmillr commented 9 years ago

@jdalton where are all github repos for lodash methods?

appsforartists commented 9 years ago

@paulmillr - think files, not repos

jdalton commented 9 years ago

What @appsforartists said.

paulmillr wrote: where are all github repos for lodash methods?

https://github.com/lodash/lodash/tree/amd https://github.com/lodash/lodash/tree/es https://github.com/lodash/lodash/tree/npm https://github.com/lodash/lodash/tree/npm-packages

https://github.com/lodash-archive/lodash-compat/tree/amd https://github.com/lodash-archive/lodash-compat/tree/npm

https://github.com/lodash-archive/lodash-node https://github.com/lodash-archive/lodash-amd

paulmillr commented 9 years ago

hmm interesting

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

monolithed commented 9 years ago

+1

gobwas commented 9 years ago

+1

monolithed commented 9 years ago

It would be great to have the following bundles

Math String Number Promise RegExp Array Object Reflect Collection Symbol

ljharb commented 9 years ago

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