-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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: Properly type Cypress and Chai globals as properties of the global object #27540
Conversation
… correctly declared not only to be in the global scope but also to be in properties of globalThis, which they are. The change allows composers to explicitly access them through globalThis (e.g., 'globalThis.cy.mount', 'globalThis.expect'). The current typing prohibits this by incorrectly declaring that the objects in question are held in local variables of the global scope rather than in properties of globalThis.
|
35 flaky tests on run #50391 ↗︎Details:
|
Test | Artifacts | |
---|---|---|
App/Cloud Integration - Latest runs and Average duration > when no runs are recorded > shows placeholders for all visible specs |
Output
Screenshots
Video
|
cypress-origin-communicator.cy.ts • 1 flaky test • app-e2e
Test | Artifacts | |
---|---|---|
Cypress In Cypress Origin Communicator > cy.origin passivity with app interactions > passes upon test reload mid test execution |
Output
Screenshots
Video
|
e2e/origin/basic_login.cy.ts • 1 flaky test • 5x-driver-electron
Test | Artifacts | |
---|---|---|
basic login > visit primary first > logs in with idp redirect |
Output
Video
|
e2e/origin/commands/unsupported_commands.cy.ts • 1 flaky test • 5x-driver-electron
Test | Artifacts | |
---|---|---|
cy.origin unsupported commands > cy.origin() is not yet supported |
Output
Video
|
e2e/origin/commands/misc.cy.ts • 1 flaky test • 5x-driver-electron
Test | Artifacts | |
---|---|---|
cy.origin misc > .end() |
Output
Video
|
The first 5 flaky specs are shown, see all 23 specs in Cypress Cloud.
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.
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.
Thanks for the contribution! Could you add tests for this in cypress-tests.ts? Adding the following should suffice:
namespace CypressGlobalsTests {
Cypress
cy
expect
assert
window.Cypress
window.cy
window.expect
window.assert
globalThis.Cypress
globalThis.cy
globalThis.expect
globalThis.assert
}
I'll soon push a commit on to the source branch of this PR that adds the code you provided in your comment to the cypress-test.ts file verbatim. However, the JavaScript compiled from that added test code would execute with no runtime error with or without the Cypress and Chai globals type changes introduced in this PR. That being said, the TypeScript compiler would refuse to compile cypress-tests.ts in the first place without those type adjustments. So is that a good enough addition to the tests? That the added test logic, while incapable of triggering a runtime error under any circumstances, would outright prevent the compilation (and thus execution) of the test code itself without the Cypress and Chai globals type changes in question? |
Yes, we have a CI job that runs the TypeScript compiler on that file, so it stands as a test for the TypeScript types. Since it fails without these type additions, it's a perfectly good test that they're doing what we expect them to do. Thanks for adding them! |
Released in This comment thread has been locked. If you are still experiencing this issue after upgrading to |
Minor modifications to CLI types so that Cypress and Chai globals are correctly declared not only to be in the global scope but also to be in properties of globalThis, which they are. The change allows composers to reference them through the global object (e.g., 'globalThis.cy.mount', 'globalThis.expect', 'window.assert'). The current typing prohibits this by incorrectly declaring that the objects in question are held in local variables of the global scope rather than in properties of the global object.
Additional details
This small change accommodates a coding style convention that demands that values in properties of the global object be accessed via the globalThis property of that object to enhance clarity regarding the origin of the values as globals as opposed to values defined in and imported from modules.
The change has no impact on the TypeScript compiler's acceptance of code that uses Cypress globals without that convention, which is the majority of such code.
Resolving the issue entailed swapping the use of
const
forvar
in type declarations in cli/types/cypress-expect.d.ts and cli/types/cypress-global-vars.d.ts.Steps to test
Ensure that typical code that leverages Cypress like this is still is accepted by the TypeScript compiler:
Then confirm that the TypeScript compiler also issues no errors regarding code that follows the aforementioned convention like the following:
How has the user experience changed?
Before:

After:

PR Tasks
cypress-documentation
? (Little to no value in drawing attention in the documentation to the fact thatglobalThis.cy
andwindow.cy
now work just as well ascy
in TypeScript code.)type definitions
?