Skip to content

feat(material-date-fns-adapter): add date adapter for date-fns#23262

Merged
zarend merged 3 commits intoangular:masterfrom
crisbeto:date-fns-adapter
Jul 30, 2021
Merged

feat(material-date-fns-adapter): add date adapter for date-fns#23262
zarend merged 3 commits intoangular:masterfrom
crisbeto:date-fns-adapter

Conversation

@crisbeto
Copy link
Copy Markdown
Member

@crisbeto crisbeto commented Jul 28, 2021

Adds a new date adapter that supports the date-fns library.

@google-cla google-cla bot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jul 28, 2021
@crisbeto crisbeto marked this pull request as ready for review July 28, 2021 20:08
@crisbeto crisbeto requested a review from a team July 28, 2021 20:08
@crisbeto crisbeto added P2 The issue is important to a large percentage of users, with a workaround target: minor This PR is targeted for the next minor release labels Jul 28, 2021
mmalerba
mmalerba approved these changes Jul 28, 2021
View reviewed changes
src/material/core/datetime/date-adapter.ts

/** InjectionToken for datepicker that can be used to override default locale code. */
export const MAT_DATE_LOCALE = new InjectionToken<string>('MAT_DATE_LOCALE', {
export const MAT_DATE_LOCALE = new InjectionToken<{}>('MAT_DATE_LOCALE', {
Copy link
Copy Markdown
Contributor

@mmalerba mmalerba Jul 28, 2021

Choose a reason for hiding this comment

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

any reason to not just use unknown?

Copy link
Copy Markdown
Member Author

@crisbeto crisbeto Jul 28, 2021

Choose a reason for hiding this comment

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

I think that might be a breaking change, I went with {} since it's what we discussed on Monday.

@crisbeto
feat(material-date-fns-adapter): add date adapter for date-fns
75bba63 
Adds a new date adapter that supports the `date-fns` library.
jelbourn
jelbourn reviewed Jul 28, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

Overall looks good, couple of minor comments

@google-cla

This comment has been minimized.

Sign in to view
@google-cla google-cla bot added cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla and removed cla: yes PR author has agreed to Google's Contributor License Agreement labels Jul 29, 2021
@google-cla google-cla bot added cla: yes PR author has agreed to Google's Contributor License Agreement and removed cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla labels Jul 29, 2021
@crisbeto @devversion
fixup! feat(material-date-fns-adapter): add date adapter for date-fns
a1c2bfc 
Fix saucelabs unit tests and the integration test for partial
compilation.
@crisbeto crisbeto added the action: merge The PR is ready for merge by the caretaker label Jul 29, 2021
devversion
devversion approved these changes Jul 29, 2021
View reviewed changes
Copy link
Copy Markdown
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.

LGTM on the tooling changes.

@zarend
Copy link
Copy Markdown
Contributor

zarend commented Jul 30, 2021

presubmit (internal). Tests ran last night with 117 failures. A few look like flakes, and many of them are timeouts.

@zarend zarend added the presubmit failures This PR has failures in Google's internal presubmit process and cannot be immediately merged label Jul 30, 2021
@crisbeto
Copy link
Copy Markdown
Member Author

crisbeto commented Jul 30, 2021

Could we re-run it without spending more time investigating it? This is an entirely new package so I doubt that anything in Google would be broken by it. There is a small change to a type in material/core, but it's backwards-compatible.

@zarend
Copy link
Copy Markdown
Contributor

zarend commented Jul 30, 2021

kk, I kicked off a re-run 🤞

@zarend zarend removed the presubmit failures This PR has failures in Google's internal presubmit process and cannot be immediately merged label Jul 30, 2021
@zarend zarend merged commit e8dd070 into angular:master Jul 30, 2021
@princemaple
Copy link
Copy Markdown
Contributor

princemaple commented Aug 12, 2021

Hi, curious about what advantages this adapter has.

date-fns operates directly on native date object. I do have date-fns installed and used in my project. Does this adapter improve code size comparing to native adapter?

@angular-automatic-lock-bot
Copy link
Copy Markdown

angular-automatic-lock-bot bot commented Sep 12, 2021

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 Sep 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@devversion devversion devversion approved these changes

@andrewseguin andrewseguin Awaiting requested review from andrewseguin

+2 more reviewers

@jelbourn jelbourn jelbourn left review comments

@mmalerba mmalerba mmalerba approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement P2 The issue is important to a large percentage of users, with a workaround target: minor This PR is targeted for the next minor release

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@crisbeto @zarend @princemaple @jelbourn @devversion @mmalerba