This repository has been archived by the owner on Apr 14, 2023. It is now read-only.
Import tslib helpers from shared package rather than inlining. #959
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Seems like a good idea, can’t really figure a scenario where these would be used outside of one that does not inline. We might have to double check if the helpers are all covered.
I have time to do so tonight after work.
Build seems to fail due to an out of memory
benjamn
force-pushed
the
import-tslib-rather-than-inlining
branch
from
07da19e
to
a79b91f
Compare
JoviDeCroock
approved these changes
Feb 26, 2019
|
Thanks for the review @JoviDeCroock! |
benjamn
added a commit
that referenced
this pull request
Mar 4, 2019
One of the changes in #959 that may have contributed to memory usage was moving minification into the Rollup build pipeline. This commit moves it back out into its own job, which is only run as part of filesize.
benjamn
added a commit
to apollographql/react-apollo
that referenced
this pull request
Mar 5, 2019
Similar in spirit to apollographql/apollo-link#959 This inlining was first introduced in PR #2661 with the following commit: de2b5fc At the time, inlining made sense because TypeScript was injecting copies of the __extends, __rest, etc. helpers into every module that used them. Depending on the tslib package seemed undesirable because the available bundle size measurement tools (e.g. bundlephobia.com) mistakenly counted the entire tslib package against react-apollo, without acknowledging the possibility of sharing that package between multiple Apollo packages. It seemed safer to inline only the helpers we needed at the top of lib/react-apollo.esm.js. Now that we have a more holistic way to measure bundle sizes (#2839), and react-apollo works better with tree-shaking tools (#2659, #2661, #2677), we know that overall application bundle sizes benefit from sharing a single copy of the tslib helper package, even if no tree-shaking is happening. Of course, with tree-shaking, that one copy of the tslib package can be shrunk to contain just the helpers that are actually used.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Most Apollo applications use at least three link packages:
apollo-link,apollo-link-http-common, andapollo-link-http.These packages import helpers like
__extends,__assign, and__restfrom thetslibpackage.If we inline these helpers in each package, we can control how much of the
tslibpackage gets bundled (only what we use), but the helpers will be duplicated unnecessarily.By importing the helpers from
tslibas an external package, we can share one copy of thetslibpackage among all Apollo packages, and bundlers that understand ECMAScriptimportsyntax can still prune the contents of thetslibpackage to include only the helpers that are used (throughout the whole application).