[release-3.0] RxJS #5749

kamilkisiela · 2020-01-03T20:36:34Z

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

kamilkisiela · 2020-01-03T20:37:53Z

The only difference in size is when tree-shaking is off but I don't believe it's big a problem

benjamn · 2020-01-03T20:44:26Z

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?

kamilkisiela · 2020-01-03T20:45:48Z

@benjamn yeah, I didn't see the rxjs is devDependencies

kamilkisiela · 2020-01-03T22:34:36Z

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

henryqdineen · 2020-01-07T20:14:08Z

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 rxjs and apollo-client used in the same bundle
How much existing code rely on existence of Observable having map(),flatMap(), etc operators built-in?

klemenoslaj · 2020-09-05T12:17:01Z

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 rxjs and apollo-client used in the same bundle

How much existing code rely on existence of Observable having map(),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.

benjamn · 2020-09-15T15:15:33Z

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 · 2020-09-15T15:20:49Z

@benjamn I can assist or even create a PR for it. RxJS is fun

benjamn · 2020-09-15T15:37:16Z

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

kamilkisiela · 2020-09-15T15:46:45Z

@benjamn I already rebased and upgraded to rxjs 7 (latest beta)

kamilkisiela · 2020-09-15T15:49:20Z

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.

benjamn · 2020-09-15T16:06:06Z

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

this is needed because of apollo client behavior apollographql/apollo-client#5749

@abdonrd

Apollo Client currently uses the Observable implementation provided by the zen-observable npm package, since it is small and works well for our needs. Although zen-observable itself is not written in TypeScript, we use the companion package @types/zen-observable to provide types. We are actively considering switching from zen-observable to RxJS (#5749), though it remains to be seen how much of a breaking change that will be (and thus whether it makes sense for a minor or major version update). In the meantime, several issues (most recently #5961) have been opened pointing out that zen-observable effectively does not support importing its code as ECMAScript modules, even though it has a separate entry point (zen-observable/esm.js) that uses ESM syntax. This is a problem the zen-observable package could easily fix by adding a "module" field to its package.json, but @abdonrd tried to propose that change in a PR (as I requested), and has not heard back since June 2020: zenparsing/zen-observable#74 Fortunately, Apollo maintains an npm package called zen-observable-ts, which was originally created to provide TypeScript types, before @types/zen-observable was introduced. This commit revives that wrapper package, so we can make sure both CommonJS and ESM exports are supported, with full TypeScript types, until the zen-observable maintainer gets around to merging that PR. I considered forking zen-observable, but I'm happy with this wrapping strategy, since reusing zen-observable makes it easier to take advantage of any future updates to the zen-observable package. I also considered using https://www.npmjs.com/package/patch-package to apply changes to node_modules/zen-observable/package.json upon installation, but that doesn't really work unless @apollo/client bundles the zen-observable implementation into itself, since otherwise the original (unpatched) zen-observable package will continue to be installed when developers npm install @apollo/client. The patch-package tool is great, but I don't think it's meant for libraries to use. Thankfully, this time around we do not need to hand-write the TypeScript types, since they can be re-exported from @types/zen-observable. I bumped the major version of zen-observable-ts, since the older versions (used by apollo-link) still get more than two million downloads per week. The source code for the zen-observable-ts package can now be found at https://github.com/apollographql/zen-observable-ts, rather than the old https://github.com/apollographql/apollo-link monorepo.

@abdonrd

Apollo Client currently uses the Observable implementation provided by the zen-observable npm package, since it is small and works well for our needs. Although zen-observable itself is not written in TypeScript, we use the companion package @types/zen-observable to provide types. We are actively considering switching from zen-observable to RxJS (#5749), though it remains to be seen how much of a breaking change that will be (and thus whether it makes sense for a minor or major version update). In the meantime, several issues (most recently #5961) have been opened pointing out that zen-observable effectively does not support importing its code as ECMAScript modules, even though it has a separate entry point (zen-observable/esm.js) that uses ESM syntax. This is a problem the zen-observable package could easily fix by adding a "module" field to its package.json, but @abdonrd tried to propose that change in a PR (as I requested), and has not heard back since June 2020: zenparsing/zen-observable#74 Fortunately, Apollo maintains an npm package called zen-observable-ts, which was originally created to provide TypeScript types, before @types/zen-observable was introduced. This commit revives that wrapper package, so we can make sure both CommonJS and ESM exports are supported, with full TypeScript types, until the zen-observable maintainer gets around to merging that PR. I considered forking zen-observable, but I'm happy with this wrapping strategy, since reusing zen-observable makes it easier to take advantage of any future updates to the zen-observable package. I also considered using https://www.npmjs.com/package/patch-package to apply changes to node_modules/zen-observable/package.json upon installation, but that doesn't really work unless @apollo/client bundles the zen-observable implementation into itself, since otherwise the original (unpatched) zen-observable package will continue to be installed when developers npm install @apollo/client. The patch-package tool is great, but I don't think it's meant for libraries to use. Thankfully, this time around we do not need to hand-write the TypeScript types, since they can be re-exported from @types/zen-observable. I bumped the major version of zen-observable-ts, since the older versions (used by apollo-link) still get more than two million downloads per week. The source code for the zen-observable-ts package can now be found at https://github.com/apollographql/zen-observable-ts, rather than the old https://github.com/apollographql/apollo-link monorepo.

benjamn · 2021-05-03T20:47:24Z

RxJS v7 was just released (as I found out from #8106) at long last, so by the logic of my comment #5749 (comment) it may be time to take another look at this PR/project.

cc @hwillson @jcreighton @brainkim for visibility

kamilkisiela · 2021-05-05T13:43:29Z

@benjamn hmmm someone took my push rights away, can't update the branch

hwillson · 2021-05-05T14:18:03Z

@kamilkisiela you should be good now - thanks!

PowerKiKi · 2021-12-17T12:05:32Z

@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 ?

benlesh · 2022-01-05T16:40:53Z

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

PowerKiKi · 2022-01-20T18:41:34Z

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.

jerelmiller · 2022-11-18T20:13:46Z

Closing in favor of #9331

kamilkisiela force-pushed the feat/rxjs branch from a1fee61 to 1b40f35 Compare January 3, 2020 20:39

kamilkisiela force-pushed the feat/rxjs branch from 1b40f35 to 24d724a Compare January 3, 2020 20:45

benjamn requested review from hwillson, benjamn and jbaxleyiii January 3, 2020 20:45

benjamn added this to the Release 3.0 milestone Jan 3, 2020

benjamn added 💡 idea 🎄 dependencies ⁉️ question 🧞‍♂️ enhancement 🧩 implementation-detail labels Jan 3, 2020

benjamn mentioned this pull request Jan 7, 2020

Remove zen-observable package #5758

Closed

hwillson changed the base branch from release-3.0 to master January 7, 2020 17:46

benjamn modified the milestones: Release 3.0, Post 3.0 Jan 23, 2020

hwillson removed their request for review June 4, 2020 11:15

MatthiasKunnen mentioned this pull request Sep 11, 2020

Fix zen-observable import #7004

Closed

kamilkisiela changed the title ~~[release-3.0] RxJS :)~~ [release-3.0] RxJS Sep 15, 2020

griest024 added a commit to griest024/daffodil that referenced this pull request Jan 13, 2021

build(all): allow synthetic default imports

f63b247

this is needed because of apollo client behavior apollographql/apollo-client#5749

griest024 added a commit to griest024/daffodil that referenced this pull request Jan 14, 2021

build(all): allow synthetic default imports

af97bc1

this is needed because of apollo client behavior apollographql/apollo-client#5749

griest024 added a commit to griest024/daffodil that referenced this pull request Jan 14, 2021

build(all): allow synthetic default imports

7c85074

this is needed because of apollo client behavior apollographql/apollo-client#5749

griest024 added a commit to griest024/daffodil that referenced this pull request Jan 20, 2021

build(all): allow synthetic default imports

8b2fb23

this is needed because of apollo client behavior apollographql/apollo-client#5749

damienwebdev pushed a commit to graycoreio/daffodil that referenced this pull request Jan 20, 2021

build(all): allow synthetic default imports (#1229)

4fae0fc

this is needed because of apollo client behavior apollographql/apollo-client#5749

benjamn mentioned this pull request Jan 25, 2021

Revive zen-observable-ts wrapper for zen-observable package. #7615

Merged

PowerKiKi mentioned this pull request Jan 25, 2021

Use zen-observable as ESM #5961

Closed

kamilkisiela force-pushed the feat/rxjs branch from 24d724a to 9cd90ca Compare May 5, 2021 14:30

kamilkisiela requested a review from StephenBarlow as a code owner May 5, 2021 14:30

kamilkisiela force-pushed the feat/rxjs branch from 9cd90ca to 5c670e5 Compare May 5, 2021 14:34

kamilkisiela added 2 commits May 5, 2021 16:35

RxJS

227d3ae

Fixes

6de7457

kamilkisiela force-pushed the feat/rxjs branch from 5c670e5 to 6de7457 Compare May 5, 2021 14:36

kamilkisiela changed the base branch from release-3.3 to main May 5, 2021 14:38

PowerKiKi mentioned this pull request Jan 20, 2022

[release-3.0] RxJS (bis) #9331

Closed

hwillson mentioned this pull request Mar 8, 2022

RxJS support #12183

Closed

hwillson modified the milestones: Post 3.0, Release 4.0 Aug 11, 2022

jpvajda removed the 🎄 dependencies label Sep 1, 2022

jerelmiller closed this Nov 18, 2022

github-actions bot locked as resolved and limited conversation to collaborators Feb 14, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[release-3.0] RxJS #5749

[release-3.0] RxJS #5749

kamilkisiela commented Jan 3, 2020 •

edited

Loading

kamilkisiela commented Jan 3, 2020

benjamn commented Jan 3, 2020 •

edited

Loading

kamilkisiela commented Jan 3, 2020

kamilkisiela commented Jan 3, 2020

henryqdineen commented Jan 7, 2020

klemenoslaj commented Sep 5, 2020

benjamn commented Sep 15, 2020

kamilkisiela commented Sep 15, 2020

benjamn commented Sep 15, 2020

kamilkisiela commented Sep 15, 2020

kamilkisiela commented Sep 15, 2020

benjamn commented Sep 15, 2020 •

edited

Loading

benjamn commented May 3, 2021

kamilkisiela commented May 5, 2021

hwillson commented May 5, 2021

PowerKiKi commented Dec 17, 2021

benlesh commented Jan 5, 2022

PowerKiKi commented Jan 20, 2022

jerelmiller commented Nov 18, 2022

[release-3.0] RxJS #5749

[release-3.0] RxJS #5749

Conversation

kamilkisiela commented Jan 3, 2020 • edited Loading

Apollo Client application bundle sizes

kamilkisiela commented Jan 3, 2020

benjamn commented Jan 3, 2020 • edited Loading

kamilkisiela commented Jan 3, 2020

kamilkisiela commented Jan 3, 2020

henryqdineen commented Jan 7, 2020

klemenoslaj commented Sep 5, 2020

benjamn commented Sep 15, 2020

kamilkisiela commented Sep 15, 2020

benjamn commented Sep 15, 2020

kamilkisiela commented Sep 15, 2020

kamilkisiela commented Sep 15, 2020

benjamn commented Sep 15, 2020 • edited Loading

benjamn commented May 3, 2021

kamilkisiela commented May 5, 2021

hwillson commented May 5, 2021

PowerKiKi commented Dec 17, 2021

benlesh commented Jan 5, 2022

PowerKiKi commented Jan 20, 2022

jerelmiller commented Nov 18, 2022

kamilkisiela commented Jan 3, 2020 •

edited

Loading

benjamn commented Jan 3, 2020 •

edited

Loading

benjamn commented Sep 15, 2020 •

edited

Loading