-
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
Remove locale from react-i18n
now that it is available in i18n-utils
#47446
Conversation
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: App Entrypoints (~103 bytes removed 📉 [gzipped])
Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used. Legend What is parsed and gzip size?Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory. Generated by performance advisor bot at iscalypsofastyet.com. |
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.
Approach lgtm.
i18n-utils
isn't yet safe to import unless you're ok with bringing in i18n-calypso
along with it. (We're trying to avoid this - why its so difficult to untangle)
^that coupling will be fixed up with #47358 which replaces i18n-calypso
's getLocaleSlug
with react-i18n
's i18nLocale
value instead.
Of course #47358 depends on your work in #47352 which is what makes the locale available in react-i18n
.
Bit of a chicken and egg situation as this PR then removes that locale from react-i18n
. I guess we need to use this new provider and set the locale along side setting the localeData
in react-i18n
in components/calypso-i18n-provider
There is a getLocaleSlug
in the i18n-calypso
that should be usable for this.
phew - round abouts but I think this will work.
Followed up on this here #47492 |
react-i18n
to i18n-utils
as it's a WordPress.com specific feature
e819415
to
6916ea7
Compare
Caution: This PR affects files in the Editing Toolkit Plugin on WordPress.com D52928-code has been created so you can easily test it on your sandbox. See this FieldGuide page about developing the Editing Toolkit Plugin for more info: PCYsg-ly5-p2 |
6916ea7
to
9994c95
Compare
react-i18n
to i18n-utils
as it's a WordPress.com specific featurereact-i18n
now that it is available in i18n-utils
Big changes since this review. This PR no longer tries to do everything and just removes i18nLocale
from the the react-i18n
package one all references to it have been removed.
The ability to get the locale slug has moved to i18n-utils since it's a Calypso specific feature.
9994c95
to
65c8422
Compare
Done by #47352 |
Changes proposed in this Pull Request
The
react-i18n
library currently supplies the current locale slug, however that package is trying to do things in a way that will work for any Gutenberg user, and there is no standard way to access the current locale slug in Gutenberg/core WP.Since #47566 the
@automattic/i18n-utils
package has had the ability to supply a locale slug, and after #47567 is merged there will no longer be any references to the locale fromreact-i18n
. So this PR removes it.i18nLocale
from theuseI18n()
hookTesting instructions
i18nLocale
after Update Gutenboarding to use new <LocaleProvider> #47567 has merged, so nothing specific to test other than smoke testing