apollo-client
apollo-client copied to clipboard
apollographql
[release-3.0] RxJS
I know zen-observable is tiny but rxjs can be too.
Apollo Client could get rid of zen-observable, @types/zen-observable and symbol-observable packages that are maintained but not as actively as rxjs. Maybe even some day we could fade away from ApolloLink and move completely towards rxjs.
Apollo Client application bundle sizes
Sample React app (deps: @apollo/client, graphql, graphql-tag, react, react-dom)
- Sizes excluding "react" and "react-dom":
| Build Tool | Tree Shaking | Minified (K) | Gzipped (K) | Δ Gzipped (K) |
|---|---|---|---|---|
| Rollup | Yes | 152 vs 149 | 41 vs 40 | +1 |
| Rollup | No | 147 vs 145 | 40 vs 39 | +1 |
- Sizes including "react" and "react-dom":
| Build Tool | Tree Shaking | Minified (K) | Gzipped (K) | Δ Gzipped (K) |
|---|---|---|---|---|
| Rollup | Yes | 280 vs 277 | 81 vs 80 | +1 |
Sample no view app (deps: @apollo/client, graphql, graphql-tag)
| Build Tool | Tree Shaking | Minified (K) | Gzipped (K) | Δ Gzipped (K) |
|---|---|---|---|---|
| Rollup | Yes | 139 vs 136 | 38 vs 37 | +1 |
| Rollup | No | 158 vs 156 | 44 vs 43 | +1 |
The only difference in size is when tree-shaking is off but I don't believe it's big a problem
Thanks for diving into this!
I've long thought it would be nice if folks could bring their own Observable implementation, but rxjs is definitely the one people ask for most often.
Looks like you might just need to update the package-lock.json file (or something along those lines; I don't really know) to get the tests to run?
@benjamn @jbaxleyiii @hwillson I can fix tests but I need to know if you guys even consider accepting the Pull Request. It seems like tests need to be be refactored a bit (observable related code, no change in the logic of tests).
I think this is a good idea and will benefit those that already have rxjs in their bundle and don't want to ship multiple Observable implementations. That being said, we don't have tree-shaking enabled yet and this will cause a bundle size regression for us. We are working towards enabling treeshaking but we have a lot of code so it won't be soon.
I like the idea of the Observable implementation being injectable but I guess in that case @apollo/client would need to implement the map() operator.
Do we have any idea about:
- How prevalent is treeshaking
- How often is
rxjsandapollo-clientused in the same bundle - How much existing code rely on existence of
Observablehavingmap(),flatMap(), etc operators built-in?
I think this is a good idea and will benefit those that already have
rxjsin their bundle and don't want to ship multiple Observable implementations. That being said, we don't have tree-shaking enabled yet and this will cause a bundle size regression for us. We are working towards enabling treeshaking but we have a lot of code so it won't be soon.I like the idea of the
Observableimplementation being injectable but I guess in that case@apollo/clientwould need to implement themap()operator.Do we have any idea about:
- How prevalent is treeshaking
- How often is
rxjsandapollo-clientused in the same bundle- How much existing code rely on existence of
Observablehavingmap(),flatMap(), etc operators built-in?
I know this is question from January, but anyway, the answer to the following question of yours is simple.
How often is rxjs and apollo-client used in the same bundle
All Angular applications using Apollo. rxjs is a first class citizen there.
However rxjs is quite popular these days so by no means is it limited to Angular alone.
Just saw this tweet from @benlesh about reducing the size of RxJS operators (see recent commits here), so I'm even more in favor of switching to RxJS once they release v7.
@kamilkisiela Also just wanted to say: we haven't forgotten about this!
@kamilkisiela It looks like there are some recent betas for v7, like https://www.npmjs.com/package/rxjs/v/7.0.0-beta.5. I know this PR has gotten pretty conflicted (sorry), but I'd be thrilled if you have time to put together an updated version, using the rxjs@7 betas.
If these changes are as backwards compatible as we hope, this could be just a minor version change for Apollo Client… though I'm also not opposed to releasing AC4 with these changes, if we think that's warranted.
the only difference between RxJS Observable and Zen is that RxJS is sync and Zen async or the other way around so it may break things, we will see.
@kamilkisiela In case this wasn't clear, be sure to rebase against main rather than master, and change the target branch of this PR to main (otherwise you'll miss the commits between AC v3.1 and v3.2+). You could also rebase against release-3.3, which is the current minor release branch we're working on.
I also updated bundle size stats. It's in general +1kb gzipped comparing to zen-observable but with Ben's PR you mention it should be the same size or less than zen-observable.
Would like to see this merged since I come more often into the issue that rxjs Observables are not of the same type as zen-observables/symbol-observable. So is there a roadmap/release date or something for 3.3?
I'm also strongly in favour of using RxJS and its observables for client-side state management and reactivity.
For the past year I've been working on a project which tries to use apollo-client as its single source of truth, but to be honest it's felt clunky and the temptation to revert to a specialised library (e.g. Redux or MobX) has been tempting.
I believe the local reactivity component of apollo-client could benefit greatly from a battle-hardened library like RxJS. Even Angular has RxJS as a mandatory dependency to manage reactivity and data streams.
RxJS v7 was just released (as I found out from https://github.com/apollographql/apollo-client/pull/8106) at long last, so by the logic of my comment https://github.com/apollographql/apollo-client/pull/5749#issuecomment-692784837 it may be time to take another look at this PR/project.
cc @hwillson @jcreighton @brainkim for visibility
@benjamn hmmm someone took my push rights away, can't update the branch
@benjamn is this still something you'd be willing to merge ?
@kamilkisiela are you able to rebase the PR ? or would we need new contributors to complete your work ?
@benjamn @kamilkisiela ...if there's anything I can do to support you all here, please let me know. FWIW, the changes here look okay. I might recommend using 7.2+, so everything can be imported from rxjs directly (instead of rxjs/operators), but that's not a blocker.
I just created #9331 in hope to revive this PR and get it merged. I would appreciate any help to finish fixing tests, and opinions about the breaking change that it might be.
