fix(data-types): unnecessary warning when getting data with DATE dataTypes by fzn0x · Pull Request #13712 · sequelize/sequelize

fzn0x · 2021-11-25T03:57:15Z

Pull Request Checklist

Please make sure to review and check all of these items:

Have you added new tests to prevent regressions?
Does npm run test or npm run test-DIALECT pass with this change (including linting)?
Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
Did you update the typescript typings accordingly (if applicable)?
Does the description below contain a link to an existing issue (Closes #[issue]) or a description of the issue you are solving?
Did you follow the commit message conventions explained in CONTRIBUTING.md?

Description Of Change

unnecessary warning when getting data with DATE dataTypes attributes.

Todos

check if value is date

sdepold · 2021-11-25T04:15:30Z

Can we have a test that ensures that the situation has improved?

fzn0x · 2021-11-25T04:17:03Z

Can we have a test that ensures that the situation has improved?

Look like checking this warning has resolve or not cannot be checked with unit test, only manual test. Correct me if I'm wrong.

fzn0x · 2021-11-25T04:18:40Z

You can check by manual test with

  const crowdFundings = await CrowdFunding.findAndCountAll({
    distinct: true, // don't count the include part https://github.com/sequelize/sequelize/pull/4016#issuecomment-116294996
    attributes: [
     'date_attributes' // <- will cause the warning from momentjs
    ],
    include: [{ all: true }],
    where: {
      ...data,
      [Op.or]: searchUtil.search(crowdFundingAttributes, query.q || ""),
    },
    ...pagination.getPaginationOptionsData(),
  });

I take this codebase from my project haha

sdepold · 2021-11-25T09:05:06Z

Is the result of the function different if it is not a date?

fzn0x · 2021-11-25T09:08:02Z

Is the result of the function different if it is not a date?

No, it's just produce momentjs warning which may some developers don't like, so I improve it to just apply timezone when the value in the correct format.

WikiRik · 2021-11-26T10:15:51Z

I don't think we need to merge this, especially if #13372 will be done soon

fzn0x · 2021-11-26T12:29:59Z

I don't think we need to merge this, especially if #13372 will be done soon

It might be implemented too but we don't have an updated PR for it. So merge this now will still be needed as minor improvement so someone not confusing with this kind of error just because we apply timezone for non-Date values.

fzn0x · 2021-11-26T12:33:01Z

Also we need to check the value first if it's date or not, whether it is using momentjs or others

WikiRik · 2021-11-26T12:55:02Z

Sure, then this looks good

* feat(typescript): create alpha release with ts * ci: add other ts versions to ci (#13686) * fix: wrong interface used within mixin (#13685) * fix(increment): fix key value broken query (#12985) Co-authored-by: Sascha Depold <[email protected]> * fix(upsert): fall back to DO NOTHING if no update key values provided (#13594) * fix(types): add instance member declaration (#13684) * fix(types): add instance member declaration * test(types): add static/instance members test cases Co-authored-by: Sascha Depold <[email protected]> * Create aws-lambda.md (#12642) * fix(docs): add aws-lamda route (#13693) * fix(types): allow override json function with custom return type (#13694) * fix(types): allow override to json function with custom return type * fix(types): remove automatic linter changes Co-authored-by: sander-mol <[email protected]> * ci(docs): add doc generation to checks (#13704) * docs: add logo (#13700) * docs: add logo * fix(docs): logo not show up (#13699) * fix(build): markdownlint * docs(readme): use internal link * docs(index.md): use internal link * docs(index): update logo rendering in docs * Center logo and headline Co-authored-by: Sascha Depold <[email protected]> Co-authored-by: Sascha Depold <[email protected]> * test: fix mocha (#13707) Co-authored-by: Rik Smale <[email protected]> Co-authored-by: Sascha Depold <[email protected]> * test: fix failing stack trace test (#13708) * test: fix failing tests (#13709) * test: fix failing tests due to minification Removes esbuild minification which was causing tests to fail and some changed behavior. * Revert "fix(upsert): fall back to DO NOTHING if no update key values provided (#13594)" This reverts commit 4071378. * fix(upsert): fall back to DO NOTHING if no update key values provided (#13711) * fix(upsert): fall back to DO NOTHING if no update key values provided (#13594) * fix: remove erroneous .length in _.isEmpty * refactor: use includes instead of or operators (#13706) * docs(contributing): add section on adding/updating deps (#13715) * docs(contributing): add Node versions Fixes #13714 * docs(contributing): add section on adding/updating deps * fix(types): add Col to where Ops (#13717) * fix(types): add Col to where Ops * fix(types): tests * fix(data-types): unnecessary warning when getting data with DATE dataTypes (#13712) * fix(data-types): unnecessary error when getting data with DATE dataTypes * fix(data-types): date stringify mariadb * fix(types): add missing schema field to sequelize options Fixes #12606 Co-authored-by: Sascha Depold <[email protected]> * fix(example): fix coordinates format as per GeoJson (#13718) * fix(example): fix coordinates format as per GeoJson * Update data-types.js Co-authored-by: Sascha Depold <[email protected]> * ci(node): use Node 16 instead of 12 (#13703) * ci(node): add Node 14 and 16 to DB tests * ci(node): move linting, test typing and release to Node 16 * ci(docs): use Node 16 for docs * ci(node): run tests on Node 10 and 16 Co-authored-by: Rik Smale <[email protected]> * refactor(postgres): move `clientMinMessages` from general to pg options (#13720) * refactor(postgres): move `clientMinMessages` from general to pg options * refactor(postgres): address review comments * refactor(postgres): address review comments * refactor(postgres): fix pipeline * fix(dialect): try to fix flaky test * fix(dialect): try to fix flaky test Co-authored-by: Jesse Peng <[email protected]> * refactor(build): use rm instead of rmdir for Node 14 and up (#13702) * fix(build): refactor rmdir to rm * refactor(build): only use rm in Node 14 and up * refactor(build): move consts inside function * refactor(build): fix definition Co-authored-by: Rik Smale <[email protected]> Co-authored-by: Sascha Depold <[email protected]> * docs(postgres): warn about deprecated clientMinMessage option (#13727) * docs(postgres): warn about deprecated clientMinMessage option * docs(deprecation-warning): fix typo in deprecation warning * docs(deprecation-warning): fix typo in deprecation warning * meta(deps): upgrade (dev)deps (#13729) Co-authored-by: Rik Smale <[email protected]> * test(integration): mark and fix flaky test (#13735) * feat(dialect): snowflake dialect support (#13406) Co-authored-by: Mohamed El Mahallawy <[email protected]> Co-authored-by: bparan <[email protected]> Co-authored-by: Matthew Blasius <[email protected]> Co-authored-by: WeRDyin <[email protected]> Co-authored-by: Marco Gonzalez <[email protected]> Co-authored-by: Constantin Metz <[email protected]> Co-authored-by: Sander Mol <[email protected]> Co-authored-by: sander-mol <[email protected]> Co-authored-by: Rik Smale <[email protected]> Co-authored-by: Fauzan <[email protected]> Co-authored-by: Rik Smale <[email protected]> Co-authored-by: AllAwesome497 <[email protected]> Co-authored-by: manish kakoti <[email protected]> Co-authored-by: Marco Kerwitz <[email protected]> Co-authored-by: Jesse Peng <[email protected]> Co-authored-by: Jesse Peng <[email protected]>

github-actions · 2021-12-04T18:31:30Z

🎉 This PR is included in version 6.12.0-beta.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

github-actions · 2021-12-17T19:26:28Z

🎉 This PR is included in version 6.12.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

moonjoungyoung · 2021-12-18T08:30:56Z

@fncolon @sdepold

Hello.
After this PR, using moment objects are throwing error.
Is this an intended modification?

fzn0x · 2021-12-20T01:24:32Z

@fncolon @sdepold

Hello. After this PR, using moment objects are throwing error. Is this an intended modification?

Hello, can you reproduce your example in an issue? technicallly it's just check a raw date value.

cmelendez · 2021-12-22T03:03:13Z

@fncolon @sdepold
Hello. After this PR, using moment objects are throwing error. Is this an intended modification?

Hello, can you reproduce your example in an issue? technicallly it's just check a raw date value.

Hi @fncolon.

We upgraded from 6.9.0 to 6.12.1 today and our app started throwing errors when using a Moment object to do equality checks. I'm not sure if this PR is the cause but it seems to be similar to the problem described by @moonjoungyoung.

Here's a piece of code that is throwing a Invalid value Moment<2021-12-21T19:52:23+00:00> error. It's part of a where clause.

phone_validation_until: {
   [Op.gte]: moment()
  },

Thanks.

cmelendez · 2021-12-22T03:18:38Z

Some extra info:

When setting any date value with moment, the stored value is 0000-00-00 00:00:00.
We are using mysql.
Moment objects works perfectly up to 6.11.0. Issues starts with >=6.12.0.

edit: pinging @sdepold

fzn0x · 2021-12-23T14:27:23Z

@fncolon @sdepold

Hello. After this PR, using moment objects are throwing error. Is this an intended modification?

Just find out the problem, it was returning date instead of formatted date, also change isDate from lodash with isMoment from moment.

fzn0x · 2021-12-23T14:27:49Z

Some extra info:

When setting any date value with moment, the stored value is 0000-00-00 00:00:00.

We are using mysql.

Moment objects works perfectly up to 6.11.0. Issues starts with >=6.12.0.

edit: pinging @sdepold

Just created the pull request in #13818, thanks!

…Types (sequelize#13712) * fix(data-types): unnecessary error when getting data with DATE dataTypes * fix(data-types): date stringify mariadb

fix(data-types): unnecessary error when getting data with DATE dataTypes

fd3ba91

fzn0x added type: bug DEPRECATED: replace with the "bug" issue type dialect: mysql For issues and PRs. Things that involve MySQL (and do not involve all dialects). labels Nov 25, 2021

fix(data-types): date stringify mariadb

dc39ae5

fzn0x changed the title ~~fix(data-types): unnecessary error when getting data with DATE dataTypes~~ fix(data-types): unnecessary warning when getting data with DATE dataTypes Nov 25, 2021

fzn0x requested a review from sdepold November 25, 2021 04:10

fzn0x assigned sdepold Nov 25, 2021

sdepold merged commit 121884b into main Nov 27, 2021

sdepold deleted the fncolon-patch-1 branch November 27, 2021 19:19

github-actions Bot added the released on @v6-beta label Dec 4, 2021

github-actions Bot added the released label Dec 17, 2021

cmelendez mentioned this pull request Dec 23, 2021

Sequelize v6.12.0+ is throwing an Error when using a moment.js object in date fields #13816

Closed

2 tasks

WikiRik mentioned this pull request Dec 26, 2021

fix(data-types): moment object throwing error #13818

Merged

7 tasks

Uh oh!

Conversation

fzn0x commented Nov 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Checklist

Description Of Change

Todos

Uh oh!

sdepold commented Nov 25, 2021

Uh oh!

fzn0x commented Nov 25, 2021

Uh oh!

fzn0x commented Nov 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sdepold commented Nov 25, 2021

Uh oh!

fzn0x commented Nov 25, 2021

Uh oh!

WikiRik commented Nov 26, 2021

Uh oh!

fzn0x commented Nov 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fzn0x commented Nov 26, 2021

Uh oh!

WikiRik commented Nov 26, 2021

Uh oh!

github-actions Bot commented Dec 4, 2021

Uh oh!

github-actions Bot commented Dec 17, 2021

Uh oh!

moonjoungyoung commented Dec 18, 2021

Uh oh!

fzn0x commented Dec 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cmelendez commented Dec 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cmelendez commented Dec 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fzn0x commented Dec 23, 2021

Uh oh!

fzn0x commented Dec 23, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

fzn0x commented Nov 25, 2021 •

edited

Loading

fzn0x commented Nov 25, 2021 •

edited

Loading

fzn0x commented Nov 26, 2021 •

edited

Loading

fzn0x commented Dec 20, 2021 •

edited

Loading

cmelendez commented Dec 22, 2021 •

edited

Loading

cmelendez commented Dec 22, 2021 •

edited

Loading