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
fix: migrating from <10 to current with typescript #24675
fix: migrating from <10 to current with typescript #24675
Conversation
Thanks for taking the time to open a PR!
|
Would a system test have caught this error? |
'node_modules/prettier/parser-flow.js', | ||
'node_modules/prettier/parser-meriyah.js', | ||
'node_modules/prettier/parser-typescript.js', | ||
'node_modules/prettier/third-party.js', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how do we know this won't change under the hood with new versions of prettier
or if we use new imports from prettier
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, we don't have a mechanism to verify this today. cypress in cypress testing does not use snapshots due to how the child cypress server is being launched. This is something I want to investigate a solution to ASAP, but for now I think it's worth getting this fix in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to be manually verified at release then for each release?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we would only need to verify it if we update the version of prettier that is being used: https://github.com/cypress-io/cypress/blob/develop/packages/data-context/package.json#L45
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll approve this in the spirit of getting the fix out however it would be nice to have tests to insure that we don't regress here in the future.
User facing changelog
Users can migrate from Cypress versions < 10 to 11 without hanging
Additional details
The prettier dependency that we use to format code in the 10 migration is not properly being rewritten during the snapshot doctoring process. I am forcing the portions of that dependency that we rely on to be force norewrite so that this no longer happens.
Steps to test
Migrate a project that uses typescript from 9 -> 11.
How has the user experience changed?
The app no longer hangs when migrating from 9 -> 11
PR Tasks
cypress-documentation
?type definitions
?