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.

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

misc: Add generic types to net-stubbing for use in intercept and wait #29508

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

scottmcginness
Copy link

  • Closes Enhance typings for cy.intercept and cy.wait #29507
    This adds a generic type to the BaseMessage interface in net-stubbing, for the body property. This then carries over to all of the related request/response types. And importantly into the cy.intercept and cy.wait methods, where it will be possible to write generic types for these and have them flow into the callbacks/chainables.

Additional details

  • Why was this change necessary?
    See the related issue for examples.

  • What is affected by this change?
    The types that are exported from net-stubbing.d.ts were almost all affected. But their generic types still default to any in all cases, so the current behaviour should remain unless the user explicitly writes the generic types.
    This may be a breaking change, depending on whether current users have specified types on these methods already. I don't think it was possible to write them in this way, but I can't quite figure out if it will be breaking for sure.

  • Any implementation details to explain?
    I'm not sure about the implementation of cy.wait(['alias1']) (i.e. the array of aliases). It seems to work, but is slightly tricky to use, because it needs to return an array of both the request and response types for each element in the alias array. As such the user has to specify cy.wait<[[Req1, Res1], [Req2, Res2]]>(['alias1', 'alias2']), which is unlike the other overloads. (As an aside, I don't have a current use case for making types on this kind of call, but it made some sense to make it possible).

Steps to test

I believe this only affects the experience inside an IDE (I used VSCode). Tested by running the dtslint script in the cli project.
I had a slightly hard time with this as I'm on Windows, and couldn't get the project to start. Nor do all the unit tests pass for me locally (but seems like just lots of line endings hopefully?)

How has the user experience changed?

Before (without types) on intercept:
image
and after:
image

Before (without types) on wait:
image
and after:
image

PR Tasks

  • Have tests been added/updated?
  • Has a PR for user-facing changes been opened in cypress-documentation? Not sure if this is required yet
  • [na] Have API changes been updated in the type definitions? These are type definitions in themselves, for another area

@CLAassistant
Copy link

CLAassistant commented May 12, 2024

CLA assistant check
All committers have signed the CLA.

@cypress-app-bot
Copy link
Collaborator

@jennifer-shehane
Copy link
Member

@scottmcginness Thanks for the contribution! Can you add a changelog entry for this change? Instructions are here.

…s-io#29507)

The nested types were apparently not supported (as written) before TypeScript ~4.1.Remove them in favour of just using `Interception<any, any>` in this overload of `wait`.
@scottmcginness
Copy link
Author

Can you add a changelog entry for this change? Instructions are here.

Thanks for the link! I've added this now, having missed that I should do it the first time around.

I noticed now that a few system tests have failed.
The related one seems to be that they are running Webpack with TypeScript 3.9, which did not support the type definitions as written for the cy.wait(string[]) overload. I've reverted how that one works, in favour of just using Interception<any, any> in this case.

If someone wants to revive the fully-typed version in future, that's fine by me. It didn't seem easy enough or worth it to get it working in all cases with that extra constraint on the TypeScript version (unless I've missed something).

Hopefully those tests will pass now 🤞

@jennifer-shehane
Copy link
Member

Rerunning the tests now

Copy link
Contributor

@cacieprins cacieprins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks fantastic! Thank you for the comprehensive tests. I've opened a cypress-io/cypress-documentation#5831 to provide examples based on this PR - can you take a look and verify that I'm understanding things correctly?

Unfortunately, we've since released a new version, so the changelog will need to be updated before we can merge it.

@scottmcginness
Copy link
Author

a cypress-io/cypress-documentation#5831 to provide examples based on this PR

This looks just like my use cases, so thank you for this. I've left some comments (hope that's alright!) just on how exactly those work.

the changelog will need to be updated

Moved the entry up to the pending section 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enhance typings for cy.intercept and cy.wait
6 participants