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

feat(intercept): support regexp matcher with middleware intercepts #16390

Merged
merged 2 commits into from May 10, 2021

Conversation

Andarist
Copy link
Contributor

@Andarist Andarist commented May 7, 2021

User facing changelog

cy.intercept accepts a RegExp matcher for url now when route matcher options are passed to it as the second argument.

Additional details

When upgrading to Cypress 7 I had to figure out how to add a delay to one of our requests. In Cypress 6 we were just adding such intercept before other intercepts but now they are matched in the reverse order.

So we had a code like this:

cy.intercept(/localization/, () => delay(500))
cy.customVisitWithSetOfCommonInterceptsInIt('')

but since this request was done immediately on startup of our application we couldn't just reorder those commands - because cy.visit only resolves after the load event.

So I've figured out that { middleware: true } can actually save me here and to my surprise this both didn't work and didn't typecheck correctly:

-cy.intercept(/localization/, () => delay(500))
+cy.intercept(/localization/, { middleware: true }, () => delay(500))

The work around this is quite simple:

-cy.intercept(/localization/, { middleware: true }, () => delay(500))
+cy.intercept('**localization**', { middleware: true }, () => delay(500))

But I think that the DX could be improved here by just allowing a regexp matcher, unless there is a strong reason why this is not supported.

How has the user experience changed?

It has been improved since now cy.intercept will accept more combination of arguments.

PR Tasks

@Andarist Andarist requested a review from a team as a code owner May 7, 2021 10:10
@Andarist Andarist requested review from flotwig and jennifer-shehane and removed request for a team May 7, 2021 10:10
@cypress-bot
Copy link
Contributor

cypress-bot bot commented May 7, 2021

Thanks for taking the time to open a PR!

@Andarist Andarist force-pushed the intercept-support-string-matcher branch from f608b14 to 57c6623 Compare May 7, 2021 11:06
Copy link
Contributor

@flotwig flotwig left a comment

Choose a reason for hiding this comment

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

But I think that the DX could be improved here by just allowing a regexp matcher, unless there is a strong reason why this is not supported.

This was definitely an oversight, not a deliberate decision. Thanks for opening a PR @Andarist! LGTM

@flotwig
Copy link
Contributor

flotwig commented May 10, 2021

I think a docs update is not necessary here, url is already documented as String | RegExp: https://docs.cypress.io/api/commands/intercept#Arguments

@flotwig flotwig merged commit a534b17 into cypress-io:develop May 10, 2021
@cypress-bot
Copy link
Contributor

cypress-bot bot commented May 10, 2021

Released in 7.3.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v7.3.0, please open a new issue.

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators May 10, 2021
@Andarist Andarist deleted the intercept-support-string-matcher branch May 11, 2021 06:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants