Skip to content

fix(common): allow to specify only some properties of DatePipeConfig#51287

Closed
PowerKiKi wants to merge 1 commit intoangular:mainfrom
PowerKiKi:optional-options
Closed

fix(common): allow to specify only some properties of DatePipeConfig#51287
PowerKiKi wants to merge 1 commit intoangular:mainfrom
PowerKiKi:optional-options

Conversation

@PowerKiKi
Copy link
Copy Markdown
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

The code already supports DatePipeConfig to have only some properties set, and not all. But the typing disallows it. This aligns the typing with the code.

Issue Number: N/A

What is the new behavior?

Can specify only some properties of DatePipeConfig

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@pullapprove pullapprove bot requested a review from AndrewKushnir August 7, 2023 04:59
The code already supports `DatePipeConfig` to have only some properties
set, and not all. But the typing disallows it. This aligns the typing
with the code.
@PowerKiKi
Copy link
Copy Markdown
Contributor Author

Should we consider this a breaking change because it could produce new TS errors (i.e. Object is possible 'null' or 'undefined') in third party code that also use this type ? or do we assume nobody is handling variables of that type ?

@JeanMeche JeanMeche added area: common Issues related to APIs in the @angular/common package breaking changes labels Aug 7, 2023
@ngbot ngbot bot modified the milestone: Backlog Aug 7, 2023
@AndrewKushnir AndrewKushnir modified the milestones: Backlog, v17-candidates Sep 7, 2023
@jessicajaniuk
Copy link
Copy Markdown
Contributor

TGP

@jessicajaniuk jessicajaniuk added the action: global presubmit The PR is in need of a google3 global presubmit label Oct 6, 2023
@jessicajaniuk
Copy link
Copy Markdown
Contributor

The TGP is green.

Copy link
Copy Markdown
Contributor

@jessicajaniuk jessicajaniuk left a comment

Choose a reason for hiding this comment

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

LGTM

reviewed-for: fw-common, public-api

@jessicajaniuk jessicajaniuk added action: merge The PR is ready for merge by the caretaker and removed action: global presubmit The PR is in need of a google3 global presubmit labels Oct 6, 2023
@pullapprove pullapprove bot requested review from alxhub and atscott October 6, 2023 17:23
@jessicajaniuk jessicajaniuk added the target: major This PR is targeted for the next major release label Oct 6, 2023
Copy link
Copy Markdown
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

@atscott
Copy link
Copy Markdown
Contributor

atscott commented Oct 6, 2023

This PR was merged into the repository by commit 85843e8.

@atscott atscott closed this in 85843e8 Oct 6, 2023
@PowerKiKi PowerKiKi deleted the optional-options branch October 7, 2023 01:49
@angular-automatic-lock-bot
Copy link
Copy Markdown

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 Nov 7, 2023
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
angular#51287)

The code already supports `DatePipeConfig` to have only some properties
set, and not all. But the typing disallows it. This aligns the typing
with the code.

PR Close angular#51287
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker area: common Issues related to APIs in the @angular/common package breaking changes target: major This PR is targeted for the next major release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants