TS: separate Window type for application under test #7806

sainthkh · 2020-06-25T02:03:43Z

Closes Correct Window type. #7856

User facing changelog

Correct type for Window object
Provide way to extend Window object type in Application Window.

Breaking change

The required minimum TypeScript version has been risen to 3.4.

Additional details

Why was this change necessary?

TypeScript users cannot use valid typescript types.

What is affected by this change?

N/A

Any implementation details to explain?

N/A

How has the user experience changed?

N/A

PR Tasks

Have tests been added/updated?
Has the original issue been tagged with a release in ZenHub?
Has a PR for user-facing changes been opened in cypress-documentation? Update typescript doc to specify minimum required TS version cypress-documentation#2945
Has a PR for changes to kitchensink been opened? feat: Add TypeScript example. cypress-example-recipes#515
Have API changes been updated in the type definitions?
[N/A] Have new configuration options been added to the cypress.schema.json?

cypress-bot · 2020-06-25T02:03:45Z

Thanks for taking the time to open a PR!

Create a Draft Pull Request if your PR is not ready for review. Mark the PR as Ready for Review when you're ready for a Cypress team member to review the PR.
Become familiar with the Code Review Checklist for guidelines on coding standards and what needs to be done before a PR can be merged.

cli/types/cypress.d.ts

bahmutov · 2020-06-26T12:32:21Z

cli/types/index.d.ts

+//                 Mike Woudenberg <https://github.com/mikewoudenberg>
+//                 Robbert van Markus <https://github.com/rvanmarkus>
+//                 Nicholas Boll <https://github.com/nicholasboll>
+// TypeScript Version: 3.4


why did the entire file change?

Interesting. I'll check if there's a change like line-ending.

Line ending was changed at d719f66. Fixed.

bahmutov

can we add an e2e test where we have a typescript fixture with ApplicationWindow interface with user property? To confirm the TS passes. Alternatively, and maybe even better is to add a recipe to cypress-example-recipes to show users how to add a property to window and have the types work out of the box.

bahmutov · 2020-06-26T12:36:08Z

cli/types/index.d.ts

+//                 Mike Woudenberg <https://github.com/mikewoudenberg>
+//                 Robbert van Markus <https://github.com/rvanmarkus>
+//                 Nicholas Boll <https://github.com/nicholasboll>
+// TypeScript Version: 3.4


can a PR to cypress-documentation for page https://on.cypress.io/typescript show the ApplicationWindow example AND minimum TypeScript version needed (3.4) in this case?

OliverJAsh · 2020-06-26T12:47:18Z

cli/types/cypress.d.ts

@@ -2001,7 +2006,7 @@ declare namespace Cypress {
    })
    ```
     */
-    window(options?: Partial<Loggable & Timeoutable>): Chainable<Window>
+    window(options?: Partial<Loggable & Timeoutable>): Chainable<Window & typeof globalThis & ApplicationWindow>


Are there other instances of Chainable<Window> that should also be updated?

cli/types/cypress.d.ts

brian-mann · 2020-06-29T18:37:53Z

Can we open an issue documenting this fix and why it's necessary. Also per @bahmutov's comments, it seems like there's a bit more work to do on this PR yeah?

OliverJAsh · 2020-06-29T18:57:25Z

I had a clear description in my original PR, but sadly that was closed: #6624

sainthkh · 2020-06-30T02:39:49Z

@OliverJAsh

It's recommended to create an issue before submitting a PR in the contributing guide.

If there is not an associated open issue, create an issue using our Issue Template.

Sometimes, ignoring guides can cause unwanted results. (And I also know that it's really tedious to read every contributing guide before contributing.)

Anyway, I opened an issue at #7856.

sainthkh · 2020-06-30T03:21:08Z

@bahmutov @brian-mann

There are 3 more things to do:

Create AUTWindow type and replace the appropriate Window types in cli/types/cypress.d.ts with it.
Create a new recipe at cypress-example-recipes to showcase out-of-the-box TypeScript, how to check typescript types and ApplicationWindow type extension => feat: Add TypeScript example. cypress-example-recipes#515
Create a new PR at https://on.cypress.io/typescript cypress-doc that the required minimum TypeScript version has been risen to 3.4

brian-mann · 2020-06-30T04:44:01Z

@sainthkh proceed ahead - we are trying to release 5.0.0 in the next day or two and want to get this in.

jennifer-shehane · 2020-06-30T05:26:32Z

@sainthkh cy.window() is not the only command to return the Window, will the other command's definitions be extended? The other commands:

cy.go()
cy.reload()
cy.visit()

jennifer-shehane · 2020-06-30T05:38:28Z

I've opened a PR for the docs to update the min required version to 3.4+ cypress-io/cypress-documentation#2945

Previously this would type error: ```ts cy.window().then(window => window.eval('1')); ```

`globalThis` was added in 3.4.

sainthkh · 2020-06-30T10:05:59Z

It seems that kitchensink needs update after #7782. (CircleCI result link)

bahmutov

I think this is good to go, once merged into v5.0-release we will add testing the recipe cypress-io/cypress-example-recipes#515 PR

sainthkh mentioned this pull request Jun 25, 2020

Correct window type #6624

Closed

5 tasks

sainthkh marked this pull request as ready for review June 25, 2020 02:26

sainthkh added the type: breaking change Requires a new major release version label Jun 25, 2020

sainthkh force-pushed the pr-6624 branch from 5026b35 to 8822edd Compare June 25, 2020 03:00

sainthkh marked this pull request as draft June 25, 2020 03:18

sainthkh marked this pull request as ready for review June 25, 2020 03:26

flotwig mentioned this pull request Jun 26, 2020

feat: v5.0 release #7753

Merged

21 tasks

jennifer-shehane requested review from bahmutov and flotwig June 26, 2020 06:11

bahmutov reviewed Jun 26, 2020

View reviewed changes

cli/types/cypress.d.ts Show resolved Hide resolved

bahmutov reviewed Jun 26, 2020

View reviewed changes

cli/types/cypress.d.ts Outdated Show resolved Hide resolved

bahmutov reviewed Jun 26, 2020

View reviewed changes

bahmutov suggested changes Jun 26, 2020

View reviewed changes

bahmutov reviewed Jun 26, 2020

View reviewed changes

OliverJAsh reviewed Jun 26, 2020

View reviewed changes

sainthkh marked this pull request as draft June 30, 2020 03:06

jennifer-shehane mentioned this pull request Jun 30, 2020

Update typescript doc to specify minimum required TS version cypress-io/cypress-documentation#2945

Merged

sainthkh force-pushed the pr-6624 branch from 8822edd to 4989f7f Compare June 30, 2020 09:34

OliverJAsh added 4 commits June 30, 2020 18:54

Correct window type

961b4cf

Previously this would type error: ```ts cy.window().then(window => window.eval('1')); ```

Upgrade minimum TS version from 2.9 to 3.4

d5a6b14

`globalThis` was added in 3.4.

Add test

8ca418f

Format

f45708f

OliverJAsh and others added 7 commits June 30, 2020 18:54

Test type

cb339ca

Expect type

1f91375

Fix syntax error

ce80b82

Add ApplicationWindow type for user-defined properties in Window object

20fa3e0

Fix comment.

5961445

crlf -> lf.

b8a76f2

Test more APIs.

43f22c5

sainthkh force-pushed the pr-6624 branch from 4989f7f to 43f22c5 Compare June 30, 2020 09:55

sainthkh mentioned this pull request Jul 1, 2020

feat: Add TypeScript example. cypress-io/cypress-example-recipes#515

Merged

jennifer-shehane requested a review from bahmutov July 6, 2020 09:11

bahmutov marked this pull request as ready for review July 6, 2020 13:35

bahmutov approved these changes Jul 6, 2020

View reviewed changes

bahmutov changed the title ~~Correct Window type.~~ TS: separate Window type for application under test Jul 6, 2020

bahmutov merged commit 9e754d5 into cypress-io:v5.0-release Jul 6, 2020

cypress-bot bot mentioned this pull request Jul 6, 2020

Correct Window type. #7856

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TS: separate Window type for application under test #7806

TS: separate Window type for application under test #7806

sainthkh commented Jun 25, 2020 •

edited by jennifer-shehane

cypress-bot bot commented Jun 25, 2020

bahmutov Jun 26, 2020

sainthkh Jun 26, 2020

sainthkh Jun 30, 2020

bahmutov left a comment

bahmutov Jun 26, 2020

OliverJAsh Jun 26, 2020

brian-mann commented Jun 29, 2020

OliverJAsh commented Jun 29, 2020

sainthkh commented Jun 30, 2020

sainthkh commented Jun 30, 2020 •

edited

brian-mann commented Jun 30, 2020

jennifer-shehane commented Jun 30, 2020

jennifer-shehane commented Jun 30, 2020

sainthkh commented Jun 30, 2020 •

edited

bahmutov left a comment

TS: separate Window type for application under test #7806

TS: separate Window type for application under test #7806

Conversation

sainthkh commented Jun 25, 2020 • edited by jennifer-shehane

User facing changelog

Breaking change

Additional details

Why was this change necessary?

What is affected by this change?

Any implementation details to explain?

How has the user experience changed?

PR Tasks

cypress-bot bot commented Jun 25, 2020

bahmutov Jun 26, 2020

Choose a reason for hiding this comment

sainthkh Jun 26, 2020

Choose a reason for hiding this comment

sainthkh Jun 30, 2020

Choose a reason for hiding this comment

bahmutov left a comment

Choose a reason for hiding this comment

bahmutov Jun 26, 2020

Choose a reason for hiding this comment

OliverJAsh Jun 26, 2020

Choose a reason for hiding this comment

brian-mann commented Jun 29, 2020

OliverJAsh commented Jun 29, 2020

sainthkh commented Jun 30, 2020

sainthkh commented Jun 30, 2020 • edited

brian-mann commented Jun 30, 2020

jennifer-shehane commented Jun 30, 2020

jennifer-shehane commented Jun 30, 2020

sainthkh commented Jun 30, 2020 • edited

bahmutov left a comment

Choose a reason for hiding this comment

sainthkh commented Jun 25, 2020 •

edited by jennifer-shehane

sainthkh commented Jun 30, 2020 •

edited

sainthkh commented Jun 30, 2020 •

edited