Skip to content
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

feat(common): Add anIntl implementation for the i18n sub-system. #55283

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

JeanMeche
Copy link
Member

@JeanMeche JeanMeche commented Apr 10, 2024

@angular-robot angular-robot bot added the detected: feature PR contains a feature commit label Apr 10, 2024
@JeanMeche JeanMeche force-pushed the poc-intl-date branch 2 times, most recently from 6c939c3 to 0ceb875 Compare April 10, 2024 15:02
@angular-robot angular-robot bot added the detected: deprecation PR contains a commit with a deprecation label Apr 10, 2024
@JeanMeche JeanMeche changed the title feat(common): Add support for i18N using the Intl API feat(common): Add and enable the Intl implementation for the i18n sub-system. Apr 10, 2024
@JeanMeche JeanMeche changed the title feat(common): Add and enable the Intl implementation for the i18n sub-system. feat(common): Add and enable the Intl implementation for the i18n sub-system. Apr 10, 2024
@JeanMeche JeanMeche added feature Issue that requests a new feature breaking changes area: i18n and removed detected: feature PR contains a feature commit labels Apr 11, 2024
@ngbot ngbot bot added this to the Backlog milestone Apr 11, 2024
@JeanMeche JeanMeche added the action: review The PR is still awaiting reviews from at least one requested reviewer label Apr 11, 2024
@angular-robot angular-robot bot added the detected: feature PR contains a feature commit label Apr 11, 2024
@alxhub alxhub self-assigned this Apr 11, 2024
@angular-robot angular-robot bot added the detected: breaking change PR contains a commit with a breaking change label Apr 12, 2024
@JeanMeche JeanMeche force-pushed the poc-intl-date branch 2 times, most recently from 767753e to 1ca64d6 Compare April 12, 2024 10:27
@JeanMeche JeanMeche marked this pull request as ready for review April 12, 2024 10:42
JeanMeche added a commit to JeanMeche/angular that referenced this pull request Apr 16, 2024
…tation for Date formatting

angular#55283 introduced an Intl implementation for the i18n sub-system. For dates, the change might be considered breaking.

This migration schematics aims to migrate every existing app, to opt-out of the new `Intl` based implementation to smooth out the transition to the new implementation.

A TODO is added to invite developers to try-out themselves the new implementation.
@angular-robot angular-robot bot removed the detected: breaking change PR contains a commit with a breaking change label Apr 18, 2024
@angular-robot angular-robot bot removed the detected: deprecation PR contains a commit with a deprecation label Apr 25, 2024
@JeanMeche JeanMeche changed the title feat(common): Add and enable the Intl implementation for the i18n sub-system. feat(common): Add anIntl implementation for the i18n sub-system. Apr 25, 2024
@JeanMeche JeanMeche added the target: minor This PR is targeted for the next minor release label Apr 25, 2024
@angular-robot angular-robot bot removed the detected: breaking change PR contains a commit with a breaking change label Apr 25, 2024
JeanMeche added a commit to JeanMeche/angular that referenced this pull request Apr 25, 2024
Ahead of angular#55283 and with what we learned from it,  let's improve our test coverage on the i18n subsystem.
AndrewKushnir pushed a commit that referenced this pull request Apr 25, 2024
Ahead of #55283 and with what we learned from it,  let's improve our test coverage on the i18n subsystem.

PR Close #55532
AndrewKushnir pushed a commit that referenced this pull request Apr 25, 2024
Ahead of #55283 and with what we learned from it,  let's improve our test coverage on the i18n subsystem.

PR Close #55532
AndrewKushnir pushed a commit that referenced this pull request Apr 25, 2024
Ahead of #55283 and with what we learned from it,  let's improve our test coverage on the i18n subsystem.

PR Close #55532
@angular-robot angular-robot bot added the detected: breaking change PR contains a commit with a breaking change label Apr 25, 2024
@JeanMeche JeanMeche force-pushed the poc-intl-date branch 2 times, most recently from 6d1e085 to b37e564 Compare April 25, 2024 20:56
Copy link
Member

@devversion devversion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review of currency formatting + plural


afterAll(() => ɵunregisterLocaleData());
// Following ignore is to ease the review of the diff
// prettier-ignore
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a follow-up to remove this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll fix the formatting once the PR is merge.

packages/common/src/i18n/format_number.ts Outdated Show resolved Hide resolved
packages/common/src/i18n/format_number.ts Outdated Show resolved Hide resolved
goldens/public-api/common/index.md Outdated Show resolved Hide resolved
packages/common/test/i18n/format_number_spec.ts Outdated Show resolved Hide resolved
packages/common/src/i18n/intl_helpers.ts Outdated Show resolved Hide resolved
packages/common/src/i18n/intl_helpers.ts Outdated Show resolved Hide resolved
packages/common/src/i18n/intl_helpers.ts Outdated Show resolved Hide resolved
packages/common/src/i18n/intl_helpers.ts Outdated Show resolved Hide resolved
@@ -90,18 +92,21 @@ export function getLocaleCurrencyCode(locale: string): string|null {
* @see {@link NgPlural}
* @see [Internationalization (i18n) Guide](guide/i18n)
*/
export function getLocalePluralCase(locale: string): (value: number) => number {
const data = findLocaleData(locale);
return data[LocaleDataIndex.PluralCase];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a breaking change? i.e. there isn't even a way to get back to legacy CLDR here AFAICT

Copy link
Member Author

@JeanMeche JeanMeche May 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TGP returned green for this.

Here is the support form Intl.PluralRules.supportedLocalesOf, it is widely within the scope of our supported browser versions.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/PluralRules/supportedLocalesOf

Depending on how strict we are on our breaking change policy, I can be considered breaking or non-breaking. Happy to know what you (and others) think about this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does Google3 actually use the CLDR plural data? I don't know off hand.

@JeanMeche JeanMeche force-pushed the poc-intl-date branch 3 times, most recently from 57687bb to 51ed5fe Compare May 5, 2024 13:50
…nd currencies

This commit adds an `Intl` implementation to format numbers, percents and currencies.
The implementation is based on the `Intl` API and allows to drop the static imports for the locale date files.

fixes angular#33803
With this commit, plurals formatting does not depend on the CLDR locale data anymore.
@JeanMeche JeanMeche force-pushed the poc-intl-date branch 2 times, most recently from 30a12e6 to 344b8d2 Compare May 8, 2024 00:16
With this commit, the Intl implementation because the default one and doesn't require the CLDR locale data imports anymore.

Opt-out is possible by invoking `useLegacyDateFormatting()` ahead of bootstrap.

BREAKING CHANGE: Some custom date format aren't supported any more (`cccccc`, `EEEEEE`, `aaaaa`, `b` to `bbbbb` and `B` to `BBBBB`)
The `DatePipe` will not support offset timezone anymore, use IANA timezones instead .
@angular-robot angular-robot bot added the area: build & ci Related the build and CI infrastructure of the project label May 8, 2024
@JeanMeche JeanMeche force-pushed the poc-intl-date branch 3 times, most recently from 323772d to 42b4fa9 Compare May 9, 2024 18:33
@angular-robot angular-robot bot removed the area: build & ci Related the build and CI infrastructure of the project label May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action: review The PR is still awaiting reviews from at least one requested reviewer area: i18n detected: breaking change PR contains a commit with a breaking change detected: feature PR contains a feature commit feature Issue that requests a new feature target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use platform Intl APIs in Angular's i18n subsystem
5 participants