Use closure-net as a dependency of Firestore #8190
Conversation
🦋 Changeset detectedLatest commit: d46e014 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Size Report 1Affected Products
Test Logs |
Size Analysis Report 1This report is too large (179,942 characters) to be displayed here in a GitHub comment. Please use the below link to see the full report on Google Cloud Storage.Test Logs |
* convert webchannel-wrapper package to use rollup * clean up rollup config * formatting
Changeset File Check ✅
|
| * "main" field to point to. | ||
| */ | ||
|
|
||
| export default {}; |
There was a problem hiding this comment.
I assume you considered this. Why can't this file re-export FetchXmlHttpFactory, Stat, MD5, and Integer, so the imports don't have to change?
There was a problem hiding this comment.
I remember the rationale was that one package only need bloom-blob and another package needed both, so this would allow the consumer that only needs bloom to avoid bringing in webchannel code, although I guess with the correct build tool config, you can tree shake from the same package. I don't remember what the 2 consumers were, I guess browser vs non-browser bundles?
Edit: I don't think you can tree-shake in a way to separate individual object within the bloom-blob or webchannel-blob from each other since they're minified and all wrapped in an IIFE? But you should be able to bring in only bloom-blob code or only webchannel-wrapper code since they're separate files entirely.
There was a problem hiding this comment.
+1. the thinking was that one package (i.e. firestore) needs both blobs whereas another package (another firebase product) will only need the webchannel blob. If tree-shaking can't be done then this is the cleaner way to go.
There was a problem hiding this comment.
Are you able to rely on your customers having tree-shaking configured (or even any particular bundler setup)?
When I prototyped this, I actually made an entirely new local package for the bloom blobs thinking this would be the only way to avoid shipping the bloom code unnecessarily. It might even be feasible to ask your build tools to integrate that local package into the relevant firebase packages (instead of having another package to publish to the registry that is supposed to be firebase-internal).
There was a problem hiding this comment.
I'm not sure why we weren't bundling webchannel-wrapper code to begin with but I think it's worth exploring if we can bundle it in the future, we should make a note to look into this later. I think it's fine to roll with keeping the separate package for now until we are able to fully explore any possible problems with bundling.
No description provided.