staltz / xstream

An extremely intuitive, small, and fast functional reactive stream library for JavaScript
http://staltz.github.io/xstream/
MIT License
2.38k stars 137 forks source link

Add ES6 module build #224

Open jvanbruegge opened 7 years ago

jvanbruegge commented 7 years ago

I'm currently profiling a cyclejs app and scope hoisting is not possible with xstream because of missing ES6 build.

The problem is again with the extra imports. The same problem we had with @cycle/time. So either:

staltz commented 7 years ago

How about import sampleCombine from 'xstream/es6/extra/sampleCombine' ?

jvanbruegge commented 7 years ago

same problem as with @cycle/time, the user has to decide, not the tools. Ideally everything would be in the same bundle, then tree shaking can do its magic

jvanbruegge commented 7 years ago

Those package/something imports are basicly just a commonjs "tree shaking" hack

staltz commented 7 years ago

I'm asking because I'm not understanding: how does making a different package xstream-extras solve the problem of "tools decide, not the user"?

jvanbruegge commented 7 years ago

Well kinda, because if you dont use extras you do not have them. But it's not a good one.

But another point. Why not include them all in the main bundle? All client bundlers (most likely either rollup or webpack) can do tree shaking with ES modules. The reason why we need a commonjs build is mainly for running the tests in node. But for the tests it does not matter if the bundle contains operators that are not used. And for the client bundle, extra operators (the ones that are not added to the Stream prototype) are minified with tree shaking

staltz commented 7 years ago

I think you're right. If anyone is serious about reducing bundle size, they'll use tree shaking or something equivalent. xstream can still be committed to small size, though, by encouraging use of a few operators, simply because prototype operators are a bit more convenient than compose-able operators.

We can do this breaking change together with the deprecation of time-based operators, currently in canary branch.

jvanbruegge commented 7 years ago

I will open a PR later

staltz commented 7 years ago

I just want to point out that your suggestion

But another point. Why not include them all in the main bundle?

Is actually issue #215. So that when making the PR we can refer to #215.

teohhanhui commented 3 years ago

Currently, xstream cannot be effectively used from jspm.dev CDN, due to the way it was bundled:

https://github.com/jspm/project/issues/83#issuecomment-724585651

rajasegar commented 3 years ago

Is is the issue resolved? I am still facing problems with rollup using xstream, can anyone please post a sample rollup config if they have figured it out...