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.

Sign up for GitHub

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

Jump to bottom

Replace Moment.js within the Cypress project. #8714

Closed
jennifer-shehane opened this issue Oct 1, 2020 · 12 comments
Closed

Replace Moment.js within the Cypress project. #8714

jennifer-shehane opened this issue Oct 1, 2020 · 12 comments
Labels
cli good first issue Good for newcomers pkg/desktop-gui This is due to an issue in the packages/desktop-gui directory pkg/driver This is due to an issue in the packages/driver directory pkg/server This is due to an issue in the packages/server directory type: breaking change Requires a new major release version type: chore Work is required w/ no deliverable to end user

Comments

@jennifer-shehane
Copy link
Member

jennifer-shehane commented Oct 1, 2020 •
edited

Current behavior

Cypress requires Moment.js in the desktop-gui, cli, driver, and server packages.

Moment.js is now considered legacy and is in maintenance mode. They recommend moving to other projects. See https://momentjs.com/docs/#/-project-status

Desired behavior

Replace Moment.js with other smaller, more up to date package(s). See their recommendations. We would be open to PRs that replace parts of Moment.js one piece at a time. Also - using newer libraries for any new Date uses is recommended.

Versions

5.3.0
@jennifer-shehane jennifer-shehane added pkg/desktop-gui This is due to an issue in the packages/desktop-gui directory cli pkg/driver This is due to an issue in the packages/driver directory pkg/server This is due to an issue in the packages/server directory good first issue Good for newcomers stage: ready for work The issue is reproducible and in scope type: chore Work is required w/ no deliverable to end user labels Oct 1, 2020
@osamajandali
Copy link

I want to start working on this 🔥

@CarlyRaeJepsenStan
Copy link

CarlyRaeJepsenStan commented Oct 2, 2020 •
edited

I can work with this! I have had experience converting my own projects from Moment.js to Day.js. I could collaborate with the other guy.

@jennifer-shehane
Copy link
Member Author

Yes we are open to PRs - even ones that partially replace some of the moment deps are fine. Check out our contributing doc.

@gvdp gvdp mentioned this issue Oct 23, 2020
Merged
5 tasks
@cypress-bot cypress-bot bot added stage: work in progress and removed stage: ready for work The issue is reproducible and in scope labels Oct 23, 2020
@jennifer-shehane jennifer-shehane mentioned this issue Nov 3, 2020
Merged
@mrmodise mrmodise mentioned this issue Nov 5, 2020
Closed
5 tasks
@cypress-bot cypress-bot bot added stage: ready for work The issue is reproducible and in scope and removed stage: work in progress labels Nov 6, 2020
@jennifer-shehane jennifer-shehane mentioned this issue Dec 1, 2020
Merged
@bahmutov bahmutov mentioned this issue Dec 3, 2020
Merged
1 task
@JonTheStone
Copy link

So will Cypress.moment still be exposed after replacing moment.js or is best practice moving forward to not use it?

@bahmutov
Copy link
Contributor

bahmutov commented Dec 8, 2020 via email

@JonTheStone
Copy link

Thanks, @bahmutov! I'll start looking into migrating to one of their recommendations.

@jennifer-shehane
Copy link
Member Author

dayjs has been pretty comparable for what we use moment for and is likely what we will move Cypress internal code to use.

@JonTheStone
Copy link

Thanks, @jennifer-shehane! dayjs it is then

@JohnnyFun
Copy link

Out of curiosity, what's the purpose of cy.[datelibrary]? Why not just load it in separately from cypress? That's what we do with dayjs and works fine. I must be missing something.

@jennifer-shehane
Copy link
Member Author

We are internally using these libraries within Cypress, so the library is already there. When we first created Cypress we just exposed the methods as a convenience. I agree that this doesn't feel as necessary anymore as it did at the time.

@jennifer-shehane jennifer-shehane mentioned this issue Jan 25, 2021
Merged
5 tasks
@jennifer-shehane jennifer-shehane added the type: breaking change Requires a new major release version label Jan 26, 2021
@cypress-bot cypress-bot bot added stage: needs review The PR code is done & tested, needs review and removed stage: ready for work The issue is reproducible and in scope labels Jan 28, 2021
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Feb 2, 2021

The code for this is done in cypress-io/cypress#14729, but has yet to be released.
We'll update this issue and reference the changelog when it's released.

@cypress-bot cypress-bot bot removed the stage: needs review The PR code is done & tested, needs review label Feb 2, 2021
@cypress-io cypress-io deleted a comment from github-actions bot Feb 5, 2021
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Apr 5, 2021

Released in 7.0.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v7.0.0, please open a new issue.

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Apr 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
cli good first issue Good for newcomers pkg/desktop-gui This is due to an issue in the packages/desktop-gui directory pkg/driver This is due to an issue in the packages/driver directory pkg/server This is due to an issue in the packages/server directory type: breaking change Requires a new major release version type: chore Work is required w/ no deliverable to end user
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

chore: replace moment with dayjs in pkg/driver mrmodise/cypress
6 participants
@jennifer-shehane @bahmutov @JohnnyFun @osamajandali @CarlyRaeJepsenStan @JonTheStone