feat: Fix detached DOM errors for all Cypress commands

[BlueWinds]

• Closes Detached DOM: Update $actionability to support cy.currentSubject() #23901

User facing changelog

Fixed the majority of Detached DOM errors, and updates the error messages when they do occur pointing out the correct way to solve them.

Breaking Change

Cypress always requeries aliases when they are referenced. This can result in certain tests that use to pass failing. For example:

cy.findByTestId('popover')
  .findByRole('button', { expanded: true })
  .as('button')
  .click()

cy.get('@button')
  .should('have.attr', 'aria-expanded', 'false')

previously passed, because the initial button was collapsed when first queried, and then later expanded. However, in Cypress 12, this test fails because the alias is always requeried from the DOM, effectively resulting in the following execution:

cy.findByTestId('popover')
  .findByRole('button', { expanded: true })
  .click()

cy.findByTestId('popover')
  .findByRole('button', { expanded: true }) // A button which matches here (is expanded)...
  .should('have.attr', 'aria-expanded', 'false') // ...will never pass this assertion.

You can rewrite tests like this to be more specific; in our case, we changed the alias to be the first button rather than the unexpanded button.

cy.findByTestId('popover')
  .findAllByRole('button')
  .first()
  .as('button')

Breaking Change

If .invoke() is followed by additional commands or assertions, it will call the named function multiple times. This has the benefit that following assertions can more reliably use the functions return value.

If this behavior is undesirable - you want the function invoked only once - move the following commands / assertions to their own chain command. For example, rewrite

cy.get('input').invoke('val', 'text').type('newText')

to

cy.get('input').invoke('val', 'text')
cy.get('input').type('newText')

Breaking Change: .invoke() now throws an error if the function returns a promise. If you wish to call a method that returns a promise and wait for it to resolve, us .then() instead of .invoke() For example, rewrite

cy.wrap(myAPI)
  .invoke('makeARequest', 'http://example.com')
  .then(res => { ...handle response... })

to

cy.wrap(myAPI)
  .then(api => api.makeARequest('http://example.com'))
  .then(res => { ...handle response... })

Additional details

The breaking changes above actually happened in #23902, but they're far more heavily exposed here, and I've finally managed to articulate the breaking change in a user-digestable way.

Steps to test

How has the user experience changed?

In addition to the breaking changes mentioned above, queries are no longer displayed as "pending" in the command log while following commands are retrying; they resolve the first time they pass. New behavior:

Old behavior:

The error messages around detached DOM errors have updated significatly. Users will encounter them less frequently; the most common cases won't occur at all anymore, but when users do encounter errors, we now make specific recommendations on how to resolve the error.

There are now two detached dom error messages, fairly similar but with different verbage based on the situation in which they occur. New error messages:

Old error message:

PR Tasks

• Have tests been added/updated?
• [n/a] Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
• Has a PR for user-facing changes been opened in cypress-documentation? Queries, Detached DOM, and Retry-Ability cypress-documentation#4835 - Just starting review. Would like to not block this PR on the docs.
• Have API changes been updated in the type definitions?

[emilyrohrbough]

same question, why remove?

[BlueWinds]

Our driver codebase has a ton of duplicate code. Each command is tested as if it were entirely independent - but they're not, they share huge portions of code. Detached dom errors are part of $actionability and part of verifyUpcomingAssertions. We don't need to test it separately for every command that uses these two bits.

[emilyrohrbough]

same question, why remove?

[BlueWinds]

Same reason as above.

[emilyrohrbough]

Why did we remove this test?

[BlueWinds]

This test was badly written (I'm not throwing shade, I wrote it when I was new at the company :D ); it mocks things unnecessarily, and duplicates the tests around it. "Are we using actionability? Yes? Then we don't need to assert of every quirk of actionability in every command that uses it.

[emilyrohrbough]

I thought we were holding off on exposing this API?

[BlueWinds]

We were holding off doing it in 10.x - didn't want to release the API until we'd completed the migration and discovered any pitfalls. That was a concern while I was working in 'develop`, but I moved to a feature branch for unrelated reasons shortly after we made the decision.

My understanding is that we'll make it fully public in 12.x.

[emilyrohrbough]

Changing from cy.currentSubejct to cy.subject is breaking. Do we need to make a err message for those using who may have already updated to use cy.currentSubject to say it's been removed & they need to use cy.subject instead?

[BlueWinds]

I'd prefer to mention it in the changelog as breaking and not leave behind any cruft.

As far as I know, the only consumer of state('subject') (other than us) is cypress-testing-library, and they haven't updated yet.

[lmiller1990]

I left some comments (mostly questions) which were answered. I tried it out with some of the examples in #7306 and it seems to work great.

I don't have much else to add, but I don't think I'm deep enough in this PR for my approve to carry as much weight as the other reviewers, so I'm going to hold off ✅ for now.

Anything else I can do to move this forward @BlueWinds? Maybe kicking off the testing library discussion internally? Do you think we have bandwidth before 12.0 to do this?

[BlueWinds]

Anything else I can do to move this forward @BlueWinds? Maybe kicking off the testing library discussion internally? Do you think we have bandwidth before 12.0 to do this?

I should have the associated docs PR ready for review by end of day today, and have started talking with the community team about other plugins and how to handle working with the community over these changes. I hope to open a PR for cypress-testing-library by end of week.

With that said, I'd like to get some approvals on this and merge it in, so that we can see the results of the binary build in issue-7306 and I can work on any new tests failing there. That's my next step - approvals and merging, while working on the less technical side of the release plan.