-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: Setup tslib
& importHelpers
to improve bundle size
#10435
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
// Mock the module to ensure that setters are available for spying | ||
jest.mock('@aws-amplify/core', () => ({ | ||
...jest.requireActual('@aws-amplify/core'), | ||
})); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this related to tslib
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So from what I can tell this is kind of a known issue with Jest & TypeScript because babel will only include getters for re-exported functions (like browserOrNode
in this file). I'm not entirely sure why this was working before though. I double checked the TS configuration and experimented with setting importHelpers
through the ts-jest
configuration but it didn't seem to affect anything one way or the other.
<!-- Please make sure to read the Pull Request Guidelines: https://github.com/aws-amplify/amplify-js/blob/main/CONTRIBUTING.md#pull-requests --> #### Description of changes This change restores `tslib` dependencies that appear to have been dropped with a recent merge. #### Issue #, if available Original PR with testing details: #10435 #### Description of how you validated changes See original PR. #### Checklist <!-- Remove items that do not apply. For completed items, change [ ] to [x]. --> - [x] PR description included - [x] `yarn test` passes - [ ] Tests are [changed or added](https://github.com/aws-amplify/amplify-js/blob/main/CONTRIBUTING.md#steps-towards-contributions) - [ ] Relevant documentation is changed or added (and PR referenced) By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Description of changes
This change enables
importHelpers
in our shared build script (build.js
) and addstslib
as a dependency to all packages that use the script to improve bundle size.Note on methodology
I collected the data below by looking at the minified WebPack bundle files that each of our modules produce. While this may not reflect all use-cases, it provides a readily available & consistent metric to evaluate the impact of the change. As far as dependency de-duping & flattening for more complex apps, the numbers below should capture the worst-case where each package includes the minimum number of
tslib
dependencies that it requires without any additional de-duping that the importing app may perform.aws-amplify
analytics
api
api-graphql
api-rest
auth
cache
core
datastore
datastore-storage-adapter
geo
interactions
predictions
storage
xr
Total Δ for entire project -73.6 KiB
Note Soon-to-be deprecated UI packages, as well as the
pushnotification
package (which is not used for web & does not use the shared build script) were not included in this change.Future improvements
build.js
so that each package can have its owntsconfig.json
importHelpers
for packages where it doesn't provide any benefit such ascache
< 2.0.0
) versions oftslib
to improve dependency flattening, for example:@aws-crypto/sha256-js
@aws-sdk/credential-provider-cognito-identity
@aws-sdk/client-cognito-identity
Issue #, if available
NA
Description of how you validated changes
Integ test pass on
next-major-version/5
: https://app.circleci.com/pipelines/github/aws-amplify/amplify-js/15155/workflows/3a454e6b-c4e8-47aa-9190-359b225d4ffdChecklist
yarn test
passesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.