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.

Sign up for GitHub

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

Jump to bottom

refactor(common): Deprecate Local Data API functions #54483

Closed
wants to merge 1 commit into from
Closed

refactor(common): Deprecate Local Data API functions #54483

wants to merge 1 commit into from

Conversation

JeanMeche
Copy link
Member

@JeanMeche JeanMeche commented Feb 16, 2024 •
edited

Ahead of delegating the i18n subsystem to the Intl API, this commit introduces the intention to remove those functions from the public API.

DEPRECATION: getCurrencySymbol, getLocaleCurrencyCode, getLocaleCurrencyName, getLocaleCurrencySymbol, getLocaleDateFormat, getLocaleDateTimeFormat, getLocaleDayNames, getLocaleDayPeriods, getLocaleDirection, getLocaleEraNames, getLocaleExtraDayPeriodRules, getLocaleExtraDayPeriods, getLocaleFirstDayOfWeek, getLocaleId, getLocaleMonthNames, getLocaleNumberFormat, getLocaleNumberSymbol, getLocalePluralCase, getLocaleTimeFormat, getLocaleWeekEndRange, getNumberOfCurrencyDigits

@JeanMeche JeanMeche mentioned this pull request Feb 16, 2024
Open
@github-actions GitHub Actions
Copy link

github-actions bot commented Feb 16, 2024 •
edited

Deployed adev-preview for 9b6cedd to: https://ng-dev-previews-fw--pr-angular-angular-54483-adev-prev-rsnzp8xu.web.app

Note: As new commits are pushed to this pull request, this link is updated after the preview is rebuilt.

@JeanMeche JeanMeche force-pushed the deprecate-locale-data branch 4 times, most recently from 0a4ae0a to dbc85d3 Compare February 16, 2024 23:52
@angular-robot angular-robot bot added the detected: deprecation PR contains a commit with a deprecation label Feb 16, 2024
@JeanMeche JeanMeche force-pushed the deprecate-locale-data branch 2 times, most recently from 7e9361c to ee7bb12 Compare February 17, 2024 02:28
@JeanMeche JeanMeche force-pushed the deprecate-locale-data branch 2 times, most recently from 3f12936 to ce9f5e9 Compare February 28, 2024 19:22
@JeanMeche JeanMeche marked this pull request as ready for review February 29, 2024 22:28
@JeanMeche JeanMeche requested review from devversion and removed request for atscott and dgp1130 February 29, 2024 22:29
@JeanMeche JeanMeche added action: review The PR is still awaiting reviews from at least one requested reviewer target: minor This PR is targeted for the next minor release labels Mar 2, 2024
@pkozlowski-opensource pkozlowski-opensource added the area: common Issues related to APIs in the @angular/common package label Mar 4, 2024
@ngbot ngbot bot added this to the Backlog milestone Mar 4, 2024
devversion
devversion reviewed Mar 7, 2024
View reviewed changes
packages/common/src/i18n/locale_data_api.ts Outdated Show resolved Hide resolved
packages/common/src/i18n/locale_data_api.ts Outdated
@@ -481,6 +535,8 @@ export function getLocaleNumberFormat(locale: string, type: NumberFormatStyle):
* @see [Internationalization (i18n) Guide](/guide/i18n-overview)
*
* @publicApi
*
* @deprecated Currencies should not be determined from a locale
Copy link
Member

Choose a reason for hiding this comment

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

Why is that? Can we provide more context to our users?

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 was under the impression that this was recommended but after further investigation I can see that several languages/frameworks also provide such data. I'll look for an alternative to suggest.

Copy link
Member Author

@JeanMeche JeanMeche Mar 12, 2024 •
edited

Choose a reason for hiding this comment

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

There is no reputable js library that provides this feature nor does it seem that a tc39 proposal is in the pipe.

Let me know what you think the suggestion I added.

@JeanMeche JeanMeche force-pushed the deprecate-locale-data branch 2 times, most recently from 204c7ef to e7bfe32 Compare March 12, 2024 20:39
@JeanMeche
refactor(common): Deprecate Local Data API functions
9b6cedd 
Ahead of delegating the i18n subsystem to the `Intl` API, this commit introduce the intention to remove those functions from the public API.

DEPRECATED: `getCurrencySymbol`, `getLocaleCurrencyCode`, `getLocaleCurrencyName`, `getLocaleCurrencySymbol`, `getLocaleDateFormat`, `getLocaleDateTimeFormat`, `getLocaleDayNames`, `getLocaleDayPeriods`, `getLocaleDirection`, `getLocaleEraNames`, `getLocaleExtraDayPeriodRules`, `getLocaleExtraDayPeriods`, `getLocaleFirstDayOfWeek`, `getLocaleId`, `getLocaleMonthNames`, `getLocaleNumberFormat`, `getLocaleNumberSymbol`, `getLocalePluralCase`, `getLocaleTimeFormat`, `getLocaleWeekEndRange`, `getNumberOfCurrencyDigits`
@JeanMeche JeanMeche mentioned this pull request Apr 10, 2024
Open
@angular angular deleted a comment from github-actions bot Apr 10, 2024
@dgp1130 dgp1130 requested review from AndrewKushnir and removed request for dgp1130 April 23, 2024 17:20
alxhub
alxhub approved these changes Apr 23, 2024
View reviewed changes
Copy link
Member

@alxhub alxhub left a comment

Choose a reason for hiding this comment

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

Reviewed-for: public-api

AndrewKushnir
AndrewKushnir approved these changes Apr 23, 2024
View reviewed changes
Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

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

Reviewed-for: public-api

@pullapprove pullapprove bot requested a review from dgp1130 April 23, 2024 19:21
@AndrewKushnir AndrewKushnir added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Apr 23, 2024
@alxhub
Copy link
Member

alxhub commented Apr 23, 2024

This PR was merged into the repository by commit d34c033.

@alxhub alxhub closed this in d34c033 Apr 23, 2024
@JeanMeche JeanMeche deleted the deprecate-locale-data branch April 23, 2024 19:23
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators May 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@devversion devversion devversion left review comments

@alxhub alxhub alxhub approved these changes

@AndrewKushnir AndrewKushnir AndrewKushnir approved these changes

Assignees
No one assigned
Labels
action: merge The PR is ready for merge by the caretaker adev: preview area: common Issues related to APIs in the @angular/common package detected: deprecation PR contains a commit with a deprecation target: minor This PR is targeted for the next minor release
Projects
None yet
Milestone
v18 feature freeze candidates
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@JeanMeche @alxhub @devversion @AndrewKushnir @pkozlowski-opensource