-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
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
[l10n] Prepare iteration on number formatting #20656
[l10n] Prepare iteration on number formatting #20656
Conversation
Details of bundle changes.Comparing: 15854b9...13a3692 Details of page changes
|
Not a related question: Do this file is tree-shakable? I suppose it's not and it needs to be split on a file per locale. |
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.
Why not put the localization in another context? How does one depend on the other?
@dmtrKovalenko Looking at the output of rollup yes, it seems to be tree-shakable. Maybe we should add a test case in our size-snapshot to prevent regression. I can look into that.
@eps1lon This discussion echos back to #19542. In hindsight, a simplification of the arguments was:
We have a new use case for date/number formatting in the library. @dtassone on the data grid, I imagine we will need to format numbers and dates? How do you envision a solution to this problem? |
This is only an issue if you expose each context object. You can still expose a single config provider that renders multiple context providers. You're talking about the basic use case that has a single config for the whole app. The amount of work you do in these one-time setups is negligible since it's done once. It would only be an issue if we don't have sensible defaults. That argument is also self-evident. However, as long as context selectors are not available in react unrelated data should go into separate context slices. |
Co-Authored-By: Quoc-Anh Nguyen <[email protected]>
Co-Authored-By: Dmitriy Kovalenko <[email protected]>
Co-Authored-By: Dmitriy Kovalenko <[email protected]>
Co-Authored-By: Danica Shen <[email protected]>
d89ab28
to
2434199
Compare
2434199
to
13a3692
Compare
I have reduced the ambition of the pull request. No new API.
The idea started with https://github.com/mui-org/material-ui/pull/20543/files#diff-4f0ea089313629267bec25abe661453fR431. In the pagination components, we don't format the numbers, we should. For instance: ant-design/ant-design#19337.
I suspect that we will get reports about SSR hydration mismatch if we land the changes as is. The support of Intl.NumberFormat isn't always consistent between browsers and servers. Since node v13, the full ICU module is loaded by default: https://nodejs.org/api/intl.html#intl_internationalization_support, it helps.
A couple of alternatives it the mismatch issue gets out of hand:
en-US
logicI'm not very fond of the idea to put the localization inside the theme, especially with the
LocalizationProvider
component we have for the pickers, that we should/could bring into the core. However, maybe we should defer the problem to v5?cc @mui-org/core