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

Fixed frontend deprecation warnings from moment.js #7013

Merged

Conversation

ezraodio1
Copy link
Contributor

What type of PR is this?

  • Refactor

Description

Created Moment in ISO 8601 format instead of using the default Date() constructor.

How is this tested?

  • Unit tests (pytest, jest)
  • E2E Tests (Cypress)
  • Manually
  • N/A

Copy link
Collaborator

@eradman eradman left a comment

Choose a reason for hiding this comment

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

For reference, here is the warning the frontend tests emit:

Deprecation warning: value provided is not in a recognized RFC2822 or ISO format. moment construction falls back to js Date(), which is not reliable across all browsers and versions.
Non RFC2822/ISO date formats are discouraged. Please refer to http://momentjs.com/guides/#/warnings/js-date/ for more info.
Arguments:
[0] _isAMomentObject: true, _isUTC: false, _useUTC: false, _l: undefined, _i: 2021-01-27T00:00:01.733983944+03:00 stderr
f: undefined, _strict: undefined, _locale: [object Object]
Error:
    at Function.createFromInputFallback (/remote/bhyve1/ci/eradman/redash/node_modules/moment/moment.js:324:25)
    at configFromString (/remote/bhyve1/ci/eradman/redash/node_modules/moment/moment.js:2550:19)
    at configFromInput (/remote/bhyve1/ci/eradman/redash/node_modules/moment/moment.js:2993:13)
    at prepareConfig (/remote/bhyve1/ci/eradman/redash/node_modules/moment/moment.js:2976:13)
    at createFromConfig (/remote/bhyve1/ci/eradman/redash/node_modules/moment/moment.js:2943:44)
    at createLocalOrUTC (/remote/bhyve1/ci/eradman/redash/node_modules/moment/moment.js:3037:16)
    at createLocal (/remote/bhyve1/ci/eradman/redash/node_modules/moment/moment.js:3041:16)
    at hooks (/remote/bhyve1/ci/eradman/redash/node_modules/moment/moment.js:16:29)
    at isDateTime (/remote/bhyve1/ci/eradman/redash/client/app/services/query-result.js:117:31)
    at /remote/bhyve1/ci/eradman/redash/client/app/services/query-result.test.js:15:22
    at Object.<anonymous> (/remote/bhyve1/ci/eradman/redash/node_modules/jest-each/build/bind.js:76:13)
    at Object.asyncJestTest (/remote/bhyve1/ci/eradman/redash/node_modules/jest-jasmine2/build/jasmineAsyncInstall.js:102:37)
    at /remote/bhyve1/ci/eradman/redash/node_modules/jest-jasmine2/build/queueRunner.js:43:12
    at new Promise (<anonymous>)
    at mapper (/remote/bhyve1/ci/eradman/redash/node_modules/jest-jasmine2/build/queueRunner.js:26:19)
    at /remote/bhyve1/ci/eradman/redash/node_modules/jest-jasmine2/build/queueRunner.js:73:41

@ezraodio1 ezraodio1 force-pushed the fix-frontend-deprecation-warnings branch from c0a9372 to 3528232 Compare June 13, 2024 15:48
eradman

This comment was marked as resolved.

@eradman eradman changed the title Fixed frontend test deprecation warnings Fixed frontend deprecation warnings from moment.js Jun 24, 2024
@eradman
Copy link
Collaborator

eradman commented Jun 24, 2024

This change solves the deprecation warnings in the browser console as well. @ezraodio1 did some more manual testing and pointed out that the tests cover a wide range of date inputs.

This seems good to me, but we'll wait until the 2024-07 snapshot before merging

@justinclift
Copy link
Member

Awesome, this'll be a welcome fix. 😄

Copy link
Collaborator

@eradman eradman left a comment

Choose a reason for hiding this comment

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

This does solve very annoying chatter in the in the console log. Merging, but if it causes trouble we will revert.

Created Moment in ISO 8601 format instead of using
the default Date() constructor.
@eradman eradman force-pushed the fix-frontend-deprecation-warnings branch from 3528232 to 32c8624 Compare July 8, 2024 13:59
@eradman eradman enabled auto-merge (squash) July 8, 2024 13:59
@eradman eradman merged commit a37ef3b into getredash:master Jul 8, 2024
11 checks passed
@justinclift
Copy link
Member

Oops, I thought we'd merged this already. 🤦

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