fix: remove CT side effects from mount when e2e testing

[ZachJW34]

• Closes Adding a command for component mount to e2e support file causes an error #22589

User facing changelog

CT side effects from import { mount } from 'cypress/{react,vue}' are disabled when running e2e tests.

Additional details

Though we scaffold CT with the mount registration in cypress/support/component.js, a user might lift this registration to the scaffolded commands.js file, which is not specific to testing type. There are side effects associated with the mount import that are required for CT testing but they will cause errors when e2e testing.

Though we have e2e and component specific support files, users might be inclined to keep all of their custom commands in a single file as documented here. Doing so should not cause these errors to surface, so I've stubbed out the CT side effects when running e2e (and improved a few errors thrown due to not finding the root selector).

Ideally, I don't want the mount import to have side effects. I think there should be an explicit import/registration that would be used inside of the component.jssupport file, but that would be a breaking change so I've left some comments around a future refactor of this.

Steps to test

The original issue has a good reproduction and I've included a system test to verify the new behavior. You can scaffold a basic project with e2e and component testing and lift the Cypress.Commands.add('mount', ...) registration into commands.js then try to run an e2e test and the error will be thrown.

How has the user experience changed?

Before:

After:

PR Tasks

• Have tests been added/updated?
• Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
• [na] Has a PR for user-facing changes been opened in cypress-documentation?
• [na] Have API changes been updated in the type definitions?

[cypress-bot]

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.

[lmiller1990]

Two small comments.

[lmiller1990]

I wonder if we even want to log this? Does a user really want to/need to see this? Eg - if you are running e2e tests with the console open, you'll see this a lot.

[ZachJW34]

Gonna remove it. We should probably outright block the use of mount in e2e tests but that was outside of the scope of this PR so I didn't tweak it.

[lmiller1990]

Instead of adding a new project, couldn't we use an existing one that has CT and E2E and just update the support/e2e.js to import cypress/react? Just to minimize the number of example projects.

[ZachJW34]

Moved to component-tests project, keeping the system-test project bloat down is good with me

[astone123]

@ZachJW34 I'm still seeing this error when running e2e tests with this branch. My commands.js file looks like this:

import { mount } from 'cypress/react'

Cypress.Commands.add('mount', (component, options = {}) => {
    const wrapped = <div>{component}</div>
    return mount(wrapped, options)
})

Am I missing something?

[ZachJW34]

@astone123 Due to how the packages are bundled with the binary, they don't get synced with a normal yarn watch. Can you try checking out the branch and running yarn? This will rebuild all the packages, then you can try running an e2e test. I used the reporter package as it had both ct and e2e setup

[astone123]

@ZachJW34 I tried running yarn on your branch, and I see that it works for the reporter package, but I still see

No element found that matches selector [data-cy-root]. Please use the mount utils to mount it properly

In my project with that commands.js file when I try to run an e2e test

[ZachJW34]

@astone123 We chatted, testing this change with a project outside of the monorepo will resolve cypress/react from the external project and thus not have the updated changes.

[cypress]

---

Test summary

37717 • 0 • 456 • 0 • 8

---

Run details

Project	cypress
Status	Passed
Commit	e1544cc
Started	Jul 11, 2022 2:21 PM
Ended	Jul 11, 2022 2:39 PM
Duration	18:14 💡
OS	Linux Debian - 10.11
Browser	Multiple

View run in Cypress Dashboard ➡️

---

Flakiness

	commands/xhr.cy.js		4
1	... > logs request + response headers
2	... > logs Method, Status, URL, and XHR
3	... > logs Method, Status, URL, and XHR
4	... > logs response
	cypress/proxy-logging.cy.ts		1
1	Proxy Logging > request logging > xhr log has response body/status code when xhr response is logged first
This comment includes only the first 5 flaky tests. See all 8 flaky tests in the Cypress Dashboard.

---

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

[lmiller1990]

Looks good, let's get this green and ship it.

[lmiller1990]

I thought about this a bit more @ZachJW34 - the current approach might not be ideal. The fact we import at all, even if we return early, is still a side effect, since it's imported, and bundlers/preprocesses might try to do things (like analyze other imports in that module).

Example: In #22437 we do a dynamic import for react-dom depending on the react version. In E2E mode, if the supportFile does import { mount } from 'cypress/react', which it does, the default webpack preprocessor will attempt to crawl and optimize the contents of cypress/react. For React <= 17 users, react-dom/client won't exist, and webpack-preprocessor will error. Example failure.

In the cypress/webpack-dev-server, we've got special logic to handle this - but not in the default preprocessor, and I don't know we'd want to leak CT specific things into webpack-preprocessor.

This is going to be a problem, and more confusing that just raising an error.

I'll sync up w/ @ZachJW34 on this, but I think we should probably just ask users to be disciplined about separating E2E and CT commands.

We can't really predict what bundles/preprocessors will do, so users need to assume that if something is imported *at all, side effects **are ** possible. Rather than leaving them with a puzzling bundler behavior (like the E2E test I linked to exhibits) I think throwing an error is probably better, since it's at least easy to understand.