-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Fix tslib dependency for format-currency #62983
Conversation
This PR does not affect the size of JS and CSS bundles shipped to the user's browser. Generated by performance advisor bot at iscalypsofastyet.com. |
e2e787d
to
2dca95e
Compare
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.
👍
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.
Thanks for this!
@@ -5,6 +5,10 @@ All notable changes to this project will be documented in this file. | |||
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), | |||
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). | |||
|
|||
## 1.0.0 - 2022-04-25 |
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.
Did you mean 1.0.1
?
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.
Oh yes 🤦
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.
Should I create a PR for this or it can be fixed directly in trunk
?
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.
A PR sounds good 👍 We generally never touch trunk
directly here.
Happy to approve it quickly for you.
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.
Here you go
#63044
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.
Thanks @manzoorwanijk 🙌
tslib
must be a runtime dependency for all the packages that produce a CommonJS output. Otherwise there are errors as seen in #62981.Changes proposed in this Pull Request
tslib
fromdevDependencies
todependencies
forformat-currency
packagereact
frompeerDependencies
for it, because it doesn't need React at all.Testing instructions
yarn && yarn build-packages
package.json
yarn add file:<path/to/wp-calypso/packages/format-currency/>
to install the local package in your new project.index.js
yarn install
.node index.js
$1.23
Fixes #62981