-
Notifications
You must be signed in to change notification settings - Fork 6.8k
feat(moment-date-adapter): implement strict mode #16462
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(moment-date-adapter): implement strict mode #16462
Conversation
f3d9f9f
to
6d828cf
Compare
Moment supports a strict flag which solves a lot of parsing errors. Examples: | Input | Format | Forgiving | Strict | |-----------------|--------------|------------|--------------| | 1/2/2017 | M/D/YYYY | 2017-01-02 | 2017-01-02 | | 1/2/2017 | MM/DD/YYYY | 2017-01-02 | Invalid date | | Januari 1, 2017 | MMMM D, YYYY | 2017-01-01 | Invalid date | | Jan 1, 2017 | MMMM D, YYYY | 2017-01-01 | Invalid date | | 2017-1-1 | MMMM D, YYYY | 2017-01-20 | Invalid date |
6d828cf
to
8df3128
Compare
format?: MomentFormatSpecification, | ||
locale?: string, | ||
): Moment { | ||
const strict = this._options ? this._options.strict : undefined; |
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.
It looks like the this._options
is used in a few places. Consider moving into a variable since it should minify a little better.
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.
I've attempted to do so. Let me know if it needs further change since I'm not sure this is what you requested.
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.
@crisbeto, is the refactor as you requested?
src/material-moment-adapter/adapter/moment-date-adapter.spec.ts
Outdated
Show resolved
Hide resolved
useUtc has been made optional to prevent the necessity for default values.
03cc7a9
to
4bd72de
Compare
Anything holding up this PR @mmalerba? |
We run all of the PRs against Google's codebase internally to make sure that they don't break any apps or tests. This one is waiting to be run |
When will this be released as a new version, literally we need it right now :) |
This will be included in the next minor release. These generally happen ~1/month |
@mmalerba I've just updated to 8.1.3 that was release 6 days ago and I don't see the strict option in it. Am I missing something? Update: I can see here it wasn't included https://github.com/angular/components/releases/tag/8.1.3 Why would that be? Update: Actually under commits I can see it was included but when I download 8.1.3 from npm the updated code is not there..only |
@brgrz, that is due to semantic versioning. 8.1.3 is a patch release. As this pr adds a new feature, it will arrive in the next minor release. See also the label, You can read more about Angular's versioning here: https://angular.io/guide/releases |
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Moment supports a strict flag which solves a lot of parsing errors. This PR allows setting this flag.
Why
Despite specifying a format, moment is very forgiving in the date strings it perceives as valid. Examples:
Note
The moment docs state
This implementation follows the flag's default value in Moment(strict off). When Moment changes their default, the
moment-date-adapter
will follow.