fix: update cypress to Typescript 5#29568
Conversation
2 flaky tests on run #55664 ↗︎
Details:
|
|||||||||||||||||||||||||||
| Test | Artifacts | |
|---|---|---|
| ... > throws when alias property isnt just a digit |
Test Replay
|
|
waiting.cy.js • 1 flaky test • 5x-driver-chrome:beta
| Test | Artifacts | |
|---|---|---|
| ... > errors > throws when waiting for 2nd response to route |
Test Replay
|
|
Review all test suite changes for PR #29568 ↗︎
There was a problem hiding this comment.
are we not losing some coverage with this for when we can calculate the column correctly?
There was a problem hiding this comment.
Are you referring to test coverage or how code coverage calculation works?
There was a problem hiding this comment.
I took a bit of a further look into this and from what I can see we are trying to infer the row/column from the evaled chai assert/expect which seems to be slightly off from what we expect. This leads to about a line or 2 difference when opening the file in the IDE vs what is displayed in the error console. I am not too sure how to fix this to be honest since it might be hard to get the accurate stack from the evaled context.
FWIW this is the case with typescript 5 usage and cypress today. Do we want to make an issue to look into correctly calculating the user invocation stack for typescript 5 users?
There was a problem hiding this comment.
Sure, lets create an issue
There was a problem hiding this comment.
Removing this preserves the existing behavior without throwing an error - imports that are not used as values will simply be stripped, since verbatimModuleSyntax is not enabled. Is that intended?
There was a problem hiding this comment.
I think this is just oversight on my end. I need to add "verbatimModuleSyntax": true to get the expected behavior. good catch!
There was a problem hiding this comment.
I'm wondering if we will need to address this separately, since we cant use "verbatimModuleSyntax": true with import syntax inside common js modules. I wonder if we want to make an issue to reword the modules to leverage "verbatimModuleSyntax": true since there isn't anything that leads me to believe this is an issue?
There was a problem hiding this comment.
I don't think we have any polyfills or other content where this would be an issue but not entirely sure
There was a problem hiding this comment.
I wonder if we just want to pin to ~5.4.5 for the time being so we aren't subject to the breaking change in 5.5 with the removal of "importsNotUsedAsValues": "error"?
There was a problem hiding this comment.
@cacieprins I wound up going with the above comment in d3916e9
There was a problem hiding this comment.
to ensure that fs.readFileSync throws without requiring it to hit the fs, it might be a good idea to mock it here. Then you could lift some of the mocking up to the #getTSCompilerOptionsForBrowser beforeEach.
There was a problem hiding this comment.
how is cc274f7223? Should be a bit dryer
…eep importsNotUsedAsValues
cc274f7 to
0c9e81e
Compare
|
@AtofStryker There's a ts-check error. |
…n the runner webpack config
… line 4 lines (duh)
|
Released in This comment thread has been locked. If you are still experiencing this issue after upgrading to |
Additional details
Updates the Cypress monorepo to typescript
5.3.3. There are a few issues with updating to5.4that cause some issues withmksnapshot. This allows the ProjectConfigIPC to be able to interpret newly added values in typescript 5 much better, paving the way for some better typescript support for newer CT framework versionsIs this a breaking change?
Based on the microsoft devblog of breaking changes, it looks like most the breaking changes are related to the compiler API itself.
The minimum node version is now 12 (which Cypress' minimum is 18 currently).
A few options are now deprecated inside the users
tsconfig, which might warn users on compilation who are using typescript 4 with their project config while Cypress uses Typescript 5 to interpret the config. However, this will not error for users, so backwards compatibility should not be an issue since our minimum version supported is typescript 4.We likely do not want to update past version
5.5until Cypress publishes a breaking change major version as5.5removes theimportsNotUsedAsValueskey, which could break users. Cypress uses whatever typescript is shipped with the end users project, except in the case of using ts-node on the server to interpret the users cypress configuration.Bug fixes
since
createProgramis no longer called in Typescript v5 inside thewebpack-preprocessor, I have added code inside thewebpack-batteries-included-preprocessorthat checks the userstsconfigwhen we registerts-loaderon behalf of the user. IfsourceMapis configured, we opt for source maps over inline source maps . I'm not sure why this no longer happens, but figured its best to fix the issue at hand for users usingwbip. See original PR #9312Steps to test
How has the user experience changed?
PR Tasks
cypress-documentation?type definitions?