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

Add ES6 module build #224

Open
jvanbruegge opened this issue Oct 10, 2017 · 10 comments
Open

Add ES6 module build #224

jvanbruegge opened this issue Oct 10, 2017 · 10 comments

Comments

@jvanbruegge
Copy link

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:

  • offer no ES6 for extra operators (will cause problems with source maps for ES6 users)
  • split package in xstream and xstream-extras (this could also be splitted in xstream-time) so if you are using @cycle/time you do not bundle the time operators + we can deprecate it some day in the future)
@staltz
Copy link
Owner

staltz commented Oct 19, 2017

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

@jvanbruegge
Copy link
Author

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
Copy link
Author

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

@staltz
Copy link
Owner

staltz commented Oct 19, 2017

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
Copy link
Author

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
Copy link
Owner

staltz commented Oct 25, 2017

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
Copy link
Author

I will open a PR later

@staltz
Copy link
Owner

staltz commented Oct 31, 2017

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
Copy link

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

jspm/project#83 (comment)

@rajasegar
Copy link

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...

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

4 participants