Chore: migrate away from moment.js

[soryy708]

Issue Creation Checklist

• I have read the contribution guidelines

Issue Description

Moment.JS is in maintenance mode, + it's a pretty old package which doesn't play well with modern build pipelines.
I propose we migrate away from MomentJS to something else, like date-fns or something else.

Additional context

Moment.JS docs say the following:

We now generally consider Moment to be a legacy project in maintenance mode
In most cases, you should not choose Moment for new projects
There are several great options to consider using instead of Moment.

Issue Template Checklist

Is this issue dialect-specific?

• No. This issue is relevant to Sequelize as a whole.
• Yes. This issue only applies to the following dialect(s): XXX, YYY, ZZZ
• I don't know.

Would you be willing to resolve this issue by submitting a Pull Request?

• Yes, I have the time and I know how to start.
• Yes, I have the time but I don't know how to start, I would need guidance.
• No, I don't have the time, although I believe I could do it if I had the time...
• No, I don't have the time and I wouldn't even know how to start.

[imwh0im]

i agree this issue. migrate from momentjs to dayjs

[soryy708]

i agree this issue. migrate from momentjs to dayjs

Cool. Why dayjs specifically?

Moment recommends the following options:

• Luxon - 1.3m weekly dls on npm
• Day.js - 4.6m weekly dls on npm
• date-fns - 8.1m weekly dls on npm
• js-Joda - 0.2m weekly dls on npm

[imwh0im]

dayjs was almost the same as momentjs in usage.
I recommend to look at the this.
https://day.js.org/

2kb: low resource
simple: If you use Moment.js, you already know how to use Day.js. (Eazy migration)
immutable: this is helps prevent bugs and avoid long debugging sessions.
I18n: Day.js has great support for internationalization

it's not a coercion. just a suggestion. :)

[soryy708]

https://date-fns.org/

modular: works well with bundlers and supports tree shaking, which helps it be very light
immutable & pure: helps to prevent bugs and avoid long debugging sessions
I18n: date-fns has dozens of locales. Only the ones that you use will be included in your project
Simple: always have one function that does one thing.
The API is unambiguous, and there is always a single approach to a problem.
reliable: date-fns respects timezones & DST.
It follows semantic versioning so, always backward compatible.
Each build CI checks more than 650 000 examples in about 400 time zones.
fast

[imwh0im]

The 'dayjs' and 'date-fns' has differences seem to be in usage.
I'd like to contribute to this work once it's decided.

[github-actions]

This issue has been automatically marked as stale because it has been open for 7 days without activity. It will be closed if no further activity occurs. If this is still an issue, just leave a comment or remove the "stale" label. 🙂

[soryy708]

@sdepold The bot said 7 days but closed it after 5. Not fair! 😂
I'm interested in reopening this issue and contributing. Need a maintainer to decide which dates library we'd like to migrate to (if at all)

[sdepold]

The bot... Little liar :) I'll get back to you later today!

[github-actions]

This issue has been automatically marked as stale because it has been open for 14 days without activity. It will be closed if no further activity occurs within the next 14 days. If this is still an issue, just leave a comment or remove the "stale" label. 🙂

[soryy708]

@sdepold Hi! Looks like we haven't made contact.
As said previously, we need a maintainer to decide which dates library we'd like to migrate to (if at all).

[soryy708]

@sdepold Please make a decision. Which dates library do we want to migrate to (if at all)?

[WikiRik]

Personally I would go with date-fns. It manipulates the normale Date object. It is a bigger refactor than moving to Day.js but it has my personal preference

[WikiRik]

Apparently this is also mentioned in the meetings backlog, not sure why GitHub did not show that; https://github.com/sequelize/meetings/projects/1#card-73600663

[sdepold]

Any particular reasons against day.js? I don't have any strong opinion really, however I library that is 2kb and similar to the currently used lib seems relatively good to me

[WikiRik]

Any particular reasons against day.js? I don't have any strong opinion really, however I library that is 2kb and similar to the currently used lib seems relatively good to me

Nothing in particular, date-fns just seems more interesting to me. But I'm fine with either.

[bach942]

I know this discussion hasn't been commented on in a while, but I would advocate for day.js. Date-fns is great, but it can't handle timezones like moment and moment-timezone does. I just tried to convert all my moment objects over to Date objects and using date-fns to handle the manipulation part and it became a nightmare (Once your string is converted to a Date object it now has the server timezone which creates all kinds of problems when trying to format, parse, and manipulate). Day.js has it built into the package just like moment did and seems like would work better to help migrate away from moment.

[bach942]

Also if you want I could see how it goes and make a PR with the migration to day.js?

[ephys]

What about sticking with moment until native Temporal is shipped? Sounds like a lot of work for something that's going to be migrated again soon after.

[bach942]

@ephys I can get on board with the Temporal Proposal, but I didn't know what kind of time frame it was going to be on. My goal was to just help migrate from moment, which has been deprecated for a while to something more supported and stable.

[ephys]

I agree, we can't know how long it's going to be. And I'm not pro using the polyfill before Temporal gets unflagged in at least some engines (just in case). I suppose Dayjs (or alternative lib) it is

[WikiRik]

I know this discussion hasn't been commented on in a while, but I would advocate for day.js. Date-fns is great, but it can't handle timezones like moment and moment-timezone does. I just tried to convert all my moment objects over to Date objects and using date-fns to handle the manipulation part and it became a nightmare (Once your string is converted to a Date object it now has the server timezone which creates all kinds of problems when trying to format, parse, and manipulate). Day.js has it built into the package just like moment did and seems like would work better to help migrate away from moment.

Hearing this I think day.js would be better then. Looking forward to see a PR with the migration. Temporal would be nice but I don't think we can really use it until the end of 2022 (or later)

[sdepold]

Discussed in our Monthly Sequelize sync:

Let's do the drop-in replacement with day.js. Given it's only 2kb in size, we will stay with it till the Temporal API is available.
Let's also do a tiny benchmarking to see how it behaves in terms of performance.

@bach942 Would you be interested in filing a PR?

[bach942]

@sdepold yeah I can fill the PR.. When I do make the PR should I tag you @sdepold?

[sdepold]

Nice one :) Yes please ping me or add a link to the PR here :)

[bach942]

@sdepold I've been prepping to work on the story and noticed that we support moment objects being saved. This story would essentially remove that support for that (they would have to convert it to a date or string object?). Just wondering how I should proceed with this?

[sdepold]

Could we detect that this thing is a moment object and extract the respective core date object out of it?
Also, shouldn't day.js support those moment objects?

If it is too much hassle to support moment objects, we could also just drop support for that and a note on the v7 breaking changes list.

@ephys please share your code to detect the moment object :)

[bach942]

@sdepold I noticed we had lots of places that use the moment.isMoment. If we move to Dayjs we could check to see if the "typeof is an object" but couldn't use the "instance of Moment" since the class wouldn't exist in the repo anymore. Unless @ephys has a different way of detecting the Moment object.

[ephys]

It's not perfect but since Moment is not going to change anymore it may be fine. We could detect something is a moment object by doing this:

theObject.constructor.name === 'Moment' && theObject._isAMomentObject

And, if a specific method needs to be called on it, check whether it exists first:

theObject.constructor.name === 'Moment' && theObject._isAMomentObject && 'format' in theObject

But that's only if it's not too much work, otherwise we can just drop moment support. It's possible some of the places that use moment.isMoment may be dropped

One of the places where it would be easy enough to preserve moment support would be here:
https://github.com/sequelize/sequelize/blob/main/src/data-types.js#L526

[soryy708]

theObject.constructor.name === 'Moment' may introduce a breaking change.
Some users use a bundler (like Webpack) on the back end to make the bundle size small, to save costs on bandwidth and storage. Another use case would be to use something like babel for access to modern ECMAScript features, while still targetting odler NodeJS releases.
The problem is that webpack/babel may mangle the names (like as part of uglification), and the fix would require the user to change their babel/webpack configuration to migrate.

[ephys]

That's true. and checking _isAMomentObject alone is not enough imo.
Then we may as well drop moment support altogether. I have plan on implementing something that lets users serialize/deserialize a datatype to their JS type of preference which could bring back support for moment (and others) in a different way (https://gist.github.com/ephys/50a6a05be7322c64645be716fea6186a#support-for-alternative-js-types).

[soryy708]

Why was Sequelize coupled to Moment in the first place? What is the functionality we're talking about dropping support of?

[ephys]

Probably because it used to be the most popular way of handling dates in JS when this was introduced.

The functionality in question is the ability to assign a moment object to an attribute, having it be serialized and saved to the database just like if it were a Date object. (you could also use them in where I believe)

[bach942]

I think it would be good to decouple Sequelize from Moment (has to be a date object or string to save), but also I didn't want to break functionality or change the roadmap if this was something that the team wanted to keep.

[ephys]

This will be included starting with v7, so it's fine to drop moment support :)

[sdepold]

Agreed. Let's drop it!

[sdepold]

Seems that we have lost some traction here. What are we even using moment.js for? Should we just keep it as is until the Temporal API is implemented natively?

Would Temporal actually be added to older versions of Node or only in the super latest version of Node?
It's still unclear right now but in the past new features have been added to LTS as well. Even if not we could use polyfills.

The core team is NOT going to work on this anymore but rather remove the external dependency altogether once Temporal lands in Node.

[bach942]

So I was actually wondering if putting day.js for moment.js is the right path. There is some logic to apply a timezone using moment-timezone, but wondering why Sequelize does that at all. It might be better to just use Date or String classes to save data to various databases (Either using ISO or standard date formats). I started the process of conversion, but wondered why Sequelize is doing some of this logic.

[bach942]

Quick search link for some of the moment objects: https://github.com/sequelize/sequelize/search?p=1&q=moment

[ephys]

Date is really hard to use correctly. It has terrible timeZone support, it converts its input to either UTC or local timeZone based on the syntax of the string. It's too dangerous to use in a library like Sequelize.

There is some logic to apply a timezone using moment-timezone, but wondering why Sequelize does that at all

It's related to the timezone Sequelize option which does two things:

• It sets the TimeZone to use in the DBMS (SET time_zone = ...). This part is fine.
• In DataTypes.DATE, it updates the TimeZone of the object being sent to / retrieved from the DBMS. This is something that I feel should be removed. It feels like a legacy solution to workaround the terrible Date object. It's not very useful now that we have Temporal.ZonedDateTime

However, this second behavior has been part of the code base for a very long time and removing it will annoy a lot of people.

I was already planning on deprecating DataTypes.DATE for other reasons. This is an extra one. Here is what I think we should do:

• Wait until Temporal lands in at least one LTS Node version
• Deprecate DataTypes.DATE
• Add DataTypes.DATETIME & DataTypes.DATETIME.ZONED
• DataTypes.DATETIME.ZONED accepts & returns only Temporal.ZonedDateTime. It ignores the Sequelize timezone option.
• DataTypes.DATETIME accepts & returns only Temporal.PlainDateTime
• require installing a Temporal polyfill or a node version that supports temporal to use the new DataTypes.DATETIME

[soryy708]

Another option would be to:

1. Decouple Sequelize's internal mechanisms from Moment, by using a delegator design pattern
2. Write a lot of tests for the delegator, to codify the expected behavior from Moment
3. Replace the delegator's implementation (that uses Moment) with a different one (that uses something like dayjs/date-fns)
4. Ensure the tests still pass

This will do a few things:

1. It will let us migrate away from Moment, while remaining confident that we didn't break anything
2. It won't cause a breaking API change, therefore not annoying Sequelize's users (1.3m weekly downloads, a lot of users)
3. If we some day chose to deprecate DataTypes.DATE and later drop support, the timezone code will be in one place, enabling easy ripping-out

That is, assuming that the Sequelize API doesn't explicitly communicate in Moment objects.

[ephys]

Moment isn't used in Sequelize enough to warrant implementing a delegation mechanism. It's only referenced 71 times in the source code, and 87 times in tests.
The very large majority of it is internal code, which can be replaced with whichever solution.

The only part of the code that uses moment in any observable capacity is in DataTypes: It has built-in support for saving moment objects to the database. But we've already decided that this will be removed.

No matter which solution we decide to use (but it seems settled that it will be dayjs), it needs to support time zones as DataTypes.DATE needs to keep its existing behavior. This won't be a breaking change.

---

I already plan on deprecating DataTypes.DATE for reasons unrelated to this thread. I'd rather not add the "timezone conversion" logic to its replacement at all, and instead force using a class that has built-in timezone support.
Right now that's going to be Temporal. Support for other libraries can be added afterward if there is a need for it.
There won't be any timezone code to rip out from DataTypes.DATETIME.ZONED, because it won't be added in the first place, as it will not accept legacy Date at all.

---

To sum things up a bit:

• We're open to a migration to dayjs, as long as there are no observable breaking changes (outside of moment objects not being first-class citizen).
• It's unlikely that the core team will handle the migration to dayjs (unless someone volunteers).
• As soon as Temporal is available in all LTS versions of Node, we will fully migrate any usage of date/moment/dayjs in our codebase to Temporal.
• I intend on deprecating DataTypes.DATE and replacing it with a cleaner DataTypes.DATETIME.ZONED that works with Temporal (as well as other changes).
  • I plan on releasing this new DataType once Temporal lands in at least one browser/node version. Using it will require using a polyfill, but using it will be optional.
  • DataTypes.DATE won't be removed until Temporal is widely available.
  • The behavior of DataTypes.DATE must remain unchanged

[fzn0x]

I wish we can move to dayjs instead, see the reason why.

Day.js is designed to be a minimalist replacement for Moment.js, using a similar API. It is not a drop-in replacement, but if you are used to using Moment's API and want to get moving quickly, consider using Day.js.

[ephys]

As I said, we'll accept a PR to migrate to migrate to dayjs, and you're free to make one, but the final implementation will use Temporal once it's available for the simple reason that it's the built-in library

[fzn0x]

As I said, we'll accept a PR to migrate to migrate to dayjs, and you're free to make one, but the final implementation will use Temporal once it's available for the simple reason that it's the built-in library

Wise choice to choose both long run and sustainability!

[DraftProducts]

I agree that this change would be nice.
Can I take care of the PR if no one has been assigned to it yet?

[ephys]

Sure :)

[github-actions]

🎉 This issue has been resolved in version 7.0.0-alpha.13 🎉

The release is available on:

• npm package (@release-v7 dist-tag)
• GitHub release

Your semantic-release bot 📦🚀