search

apollographql / apollo-client

:rocket:  A fully-featured, production ready caching GraphQL client for every UI framework and GraphQL server.
https://apollographql.com/client
MIT License
19.38k stars 2.66k forks source link

Use zen-observable as ESM #5961

Closed abdonrd closed 3 years ago

abdonrd commented 4 years ago

The last version of zen-observable has an ESM export: esm.js.

You can also check it at unpkg: https://unpkg.com/browse/zen-observable@0.8.15/

It would be great that Apollo Client uses that instead of the CJS export (index.js).

That way, zen-observable works directly in the browser.

benjamn commented 4 years ago

Here's where we import Observable from "zen-observable". As you can see, we're not doing anything that would force your bundler to import the CommonJS entry point, and the version constraint in our package.json (^0.8.14) permits installing the latest version of zen-observable, which is just 0.8.15. In short, I don't think Apollo Client is standing in the way of using the new ESM entry point. Let me know if there's something concrete you want us to do here.

billyjanitsch commented 4 years ago

The problem is that zen-observable doesn't actually point to its ESM build in its package.json so a bundler can't know that it has one. That package would need to add "module": "esm.js" to its package.json for this to work.

I wonder if this is also the cause of https://github.com/apollographql/apollo-client/issues/5686, although I don't know what would have changed between apollo-boost and @apollo/client to cause this in one but not the other.

benjamn commented 4 years ago

ESM support is a very recent addition to zen-observable, so I'm sure they would appreciate an issue/PR to add the "module": "esm.js" entry point to package.json. You can certainly link back to this issue as supporting evidence, but I believe it's their responsibility to follow the same conventions that other CJS/ESM packages follow.

bennypowers commented 4 years ago

There's a use case of apollo-client without a bundler that's worth considering here. Buildless workflows and even smaller applications that use unbundled CDN and http2.

I believe post-3.0 ac is moving away from observable, but in the mean time it would be nice to have an all-browser option to cover those use cases.

abdonrd commented 4 years ago

@benjamn since the maintainer has not responded for a long time... Would you be interested in changing the package if I publish it under a scope? Something like @abdonrd/zen-observable. Just with this PR to use an ESM build.

benjamn commented 3 years ago

@abdonrd Thanks for following through and opening that PR! Since there's been no response, I'm moving ahead with a wrapping strategy, using the package name zen-observable-ts: #7615. If your PR ever gets merged (🤞 ), we can go back to using zen-observable directly.

These changes should be released in Apollo Client v3.4 (the next minor version).

PowerKiKi commented 3 years ago

What about finally moving to rxjs, which is a very capable, well maintained and tree shake library ?

It is so good that it is already used in Apollo client tests code (!). Moving to rxjs would unify your codebase while reducing bundle size of Angular projects, most likely not increasing React project size. And last but not least it would reduce your maintenance work.

I'm more than willing to create a pr for that, if you give me the go.

PowerKiKi commented 3 years ago

I guess the answer to my question is #5749

PS: btw, thanks benjamn for your very informative commit message. I know we don't always feel like taking extra time for that, but yours are an invaluable source of information 🎉

abdonrd commented 3 years ago

Closed by https://github.com/apollographql/apollo-client/pull/7615, right?

Just fast-json-stable-stringify as ESM to have all deps as ESM! 🤞🏻

benjamn commented 3 years ago

@abdonrd Yes! However, please note that #7615 is only in the v3.4 betas at this point.

Edit: In addition to fast-json-stable-stringify (which I have a plan to replace), there's this little package called react that does not export ESM modules, though you can avoid that by importing from @apollo/client/core.

abdonrd commented 3 years ago

@benjamn yes, looking forward to the release!

I'm very glad to read that there will be a solution for fast-json-stable-stringify! 👏🏻

"this little package called react". Yeah, little package 😂 Personally I think the best option would be to remove React from @apollo/client (that works for any) and have something like @apollo/client/react with React. But yes, for now I use @apollo/client/core.