-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[NoQA] feat: Remove moment from the project (Except Datepicker) #28175
Changes from 7 commits
189f2b9
7c18da2
503eab3
b5016aa
93813c0
71391a1
3c86b0f
0cd0b9f
8a6dd64
84b7beb
42830bc
137d9aa
7538e54
e68a84a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
import _ from 'underscore'; | ||
import moment from 'moment'; | ||
import {isAfter} from 'date-fns'; | ||
import Onyx from 'react-native-onyx'; | ||
import {AppState} from 'react-native'; | ||
import lodashGet from 'lodash/get'; | ||
|
@@ -42,7 +42,7 @@ const setExpirationTimer = () => { | |
}, REFRESH_INTERVAL); | ||
}; | ||
|
||
const hasTokenExpired = () => moment().isAfter(currentToken.expiration); | ||
const hasTokenExpired = () => isAfter(new Date(), currentToken.expiration); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is incorrect. This should be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, have you tested the format of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "I think this is incorrect. This should be isAfter(currentToken.expiration, new Date()) instead." There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you post a screenshot of two dates using moment and isAfter function? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sure, one moment There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. new Date() There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm talking about the input values themselves that you used for comparison. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Get it, will show now There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. looks correct |
||
|
||
const clearToken = () => { | ||
console.debug('[MapboxToken] Deleting the token stored in Onyx'); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
import _ from 'underscore'; | ||
import lodashGet from 'lodash/get'; | ||
import Onyx from 'react-native-onyx'; | ||
import moment from 'moment'; | ||
import {isBefore} from 'date-fns'; | ||
import ONYXKEYS from '../../ONYXKEYS'; | ||
import * as API from '../API'; | ||
import CONST from '../../CONST'; | ||
|
@@ -460,7 +460,7 @@ function isBlockedFromConcierge(blockedFromConciergeNVP) { | |
return false; | ||
} | ||
|
||
return moment().isBefore(moment(blockedFromConciergeNVP.expiresAt), 'day'); | ||
return isBefore(new Date(), new Date(blockedFromConciergeNVP.expiresAt)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is incorrect again. It should be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here is correct again: |
||
} | ||
|
||
function triggerNotifications(onyxUpdates) { | ||
|
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.
This is
yyyy-MM-dd
unlikeYYYY-MM-DD
as it was previously.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.
Yes, its the same, moment uses own formats, date-fns uses IANA and doesnt support YYYY-MM-DD
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.
Not sure about this comment, the output of the date here is same as from moment, right? @allroundexperts @waterim
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.
confirmed its the same in DM