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.

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

[T99687] BREAKING CHANGE(dayjs): Migrate moment to dayjs #150

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

edwin-tandiono
Copy link
Member

@edwin-tandiono edwin-tandiono commented May 7, 2024

Compared to moment, dayjs is significantly smaller (73.1 kB vs 3 kB minified & gzipped) while still having similar date APIs. This should help reduce client bundle size for our front end projects which use cermati utils.

This change introduces a breaking behavior regarding invalid date input format.
Previously, parsing an incomplete input date using moment will fill the missing value. dayjs doesn't have this behavior. Considering adding date-format validation if your input is not guaranteed to have a valid format.

const dateWithoutSeconds = '2024-05-07T13:00';
const dateFormatExpectingSeconds = 'YYYY-MM-DDTHH:mm:ss';

moment(dateWithoutSeconds, dateFormatExpectingSeconds).format()
> '2024-05-07T13:00:00+07:00'

dayjs(dateWithoutSeconds, dateFormatExpectingSeconds).format()
> 'Invalid Date'

@edwin-tandiono edwin-tandiono changed the title [T99687] feat(dayjs): Migrate moment to dayjs [T99687] BREAKING CHANGE(dayjs): Migrate moment to dayjs May 7, 2024
Copy link

@ray1412 ray1412 left a comment

Choose a reason for hiding this comment

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

lgtm

@nsofian
Copy link
Contributor

nsofian commented May 29, 2024

Compared to moment, dayjs is significantly smaller (73.1 kB vs 3 kB minified & gzipped) while still having similar date APIs. This should help reduce client bundle size for our front end projects which use cermati utils.

This change introduces a breaking behavior regarding invalid date input format. Previously, parsing an incomplete input date using moment will fill the missing value. dayjs doesn't have this behavior. Considering adding date-format validation if your input is not guaranteed to have a valid format.

const dateWithoutSeconds = '2024-05-07T13:00';
const dateFormatExpectingSeconds = 'YYYY-MM-DDTHH:mm:ss';

moment(dateWithoutSeconds, dateFormatExpectingSeconds).format()
> '2024-05-07T13:00:00+07:00'

dayjs(dateWithoutSeconds, dateFormatExpectingSeconds).format()
> 'Invalid Date'

I tried it still successfully parsed the string (?)

> const dateWithoutSeconds = '2024-05-07T13:00';
undefined
> const dateFormatExpectingSeconds = 'YYYY-MM-DDTHH:mm:ss';
undefined
> const dayjs = require('dayjs')
undefined
> dayjs(dateWithoutSeconds, dateFormatExpectingSeconds).format()
'2024-05-07T13:00:00+07:00'

using the same dayjs version, 1.11.10

@edwin-tandiono
Copy link
Member Author

edwin-tandiono commented May 29, 2024

I tried it still successfully parsed the string (?)

Sorry, forgot to mention that the issue occurs if we extend custom parse format (which we do) which is necessary to handle custom formats provided by the users.

const dayjs = require('dayjs');
const customParseFormat = require('dayjs/plugin/customParseFormat');

dayjs.extend(customParseFormat);

const dateWithoutSeconds = '2024-05-07T13:00';
const dateFormatExpectingSeconds = 'YYYY-MM-DDTHH:mm:ss';

console.log(dayjs(dateWithoutSeconds, dateFormatExpectingSeconds).format());

> Invalid Date

will add these details on the changelog

Copy link
Contributor

@nsofian nsofian left a comment

Choose a reason for hiding this comment

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

lgtm from me

@edwin-tandiono
Copy link
Member Author

This PR is pending due to known dayjs strict parsing bug iamkun/dayjs#929.

> dayjs('2024-05-20T00:00:00.000+07:00', 'YYYY-MM-DD[T]HH:mm:ss.SSSZ', true).isValid()
true

> dayjs('2024-05-20T00:00:00.000Z', 'YYYY-MM-DD[T]HH:mm:ss.SSSZ', true).isValid()
false

The PR is currently nor urgent as our front end projects don't have date time validator for now. We will revisit this PR once the need arises, and we'll see if there are fix or workaround by then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants