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

fix: Next.JS 12 components testing failing with TypeError: Cannot read property 'traceChild' of undefined #18648

Merged
merged 4 commits into from
Oct 27, 2021

Conversation

pbilyk
Copy link
Contributor

@pbilyk pbilyk commented Oct 26, 2021

User facing changelog

Fix component testing errors for next 12.0

Additional details

In Next.JS 12 the trace folder wass moved outside of telemetry folder. This code checks two locations now for the telemetry.js. If it doesn't find it anywhere then it's no-op.

How has the user experience changed?

Component testing now works for both Next 11 and Next 12

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Oct 26, 2021

Thanks for taking the time to open a PR!

@CLAassistant
Copy link

CLAassistant commented Oct 26, 2021

CLA assistant check
All committers have signed the CLA.

@pbilyk pbilyk marked this pull request as ready for review October 26, 2021 19:48
Copy link
Contributor

@lmiller1990 lmiller1990 left a comment

Choose a reason for hiding this comment

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

Cool, thanks for the fix!

Looks like upgrading to Next.js 12 pulls in React 17, which bundles react/jsx-runtime iirc, but the rest of our deps are pinned to React 16, which doesn't have the jsx runtime bundled (could be wrong about the actual cause of the CI failure, just a guess based on previous experience).

We will need to into a way to support this, and test against Next.js versions 10, 11, 12. Alternatively, I think we can probably drop Next.js 10 support soonish, since it is two major versions behind.

@ZachJW34 and I can look into this. I'd say this is pretty high priority.

@pbilyk
Copy link
Contributor Author

pbilyk commented Oct 27, 2021

Thanks! Let me know if I can assist you in any way :)

Copy link
Contributor

@ZachJW34 ZachJW34 left a comment

Choose a reason for hiding this comment

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

@pbilyk Thanks for the contribution! Let's revert your latest commit and go with your initial solution. That way we can provide support for all versions of Next. We can just ts-ignore any of the typescript errors that might originate from trying to import Next v12 code.

A few things that I found a bit strange when trying to test this. For Next v12, the error is silent. I can confirm that your fix works, but it no longer shows the cannot read property 'traceChild' of undefined error that I expected to see without your changes. Instead, webpack-dev-server exits and that's that. It would still be nice if you had a reproduction of this error for Next v12, maybe I'm doing something strange.

Screen Shot 2021-10-27 at 11 28 01 AM

Also, I had to disable the new Rust compiler optimizations to get Webpack to successfully compile. I'm curious if you saw anything similar when you were testing.

Edit:
Here is a branch that I used to test this if it helps. You clone the repo, checkout the branch and cd into create-next-app-12. Running npx cypress open-ct fails silently. You can then build @cypress/react with yarn workspace @cypress/react build and copy the contents of dist into the local node_modules of create-next-app-12.

npm/react/plugins/next/getRunWebpackSpan.ts Show resolved Hide resolved
Copy link
Contributor

@ZachJW34 ZachJW34 left a comment

Choose a reason for hiding this comment

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

Looks good. LGTM!

@ZachJW34 ZachJW34 merged commit cb0cbdf into cypress-io:master Oct 27, 2021
@ZachJW34
Copy link
Contributor

@pbilyk can you try adding "nodeVersion": "system" to your cypress.json and run without babel (so as to enable the Rust optimizations)?

@pbilyk
Copy link
Contributor Author

pbilyk commented Oct 29, 2021

@ZachJW34 Yay! It works!

Edit: I mean end to end works. The fix for component testing is not yet released as far as I understand 😀

@ZachJW34
Copy link
Contributor

@pbilyk I'll try and get it released tomorrow!

@ZachJW34
Copy link
Contributor

This was released in 5.10.2.

tgriesser added a commit that referenced this pull request Nov 3, 2021
* develop: (40 commits)
  fix(driver): Sticky elements within a fixed container will not prevent an element from being scrolled to (#18441)
  chore: make `create` function on server.ts obsolete (#18615)
  docs: Add instructions to squash commits to develop in Contributing (#18728)
  fix(@cypress/react): throw if using Next.js swc-loader without nodeVersion=system (#18686)
  refactor: remove Ramda (#18723)
  chore: Increase paralleled machines for desktop-gui tests (#18725)
  chore: Update Chrome (stable) to 95.0.4638.69 (#18696)
  chore: release @cypress/vue-v3.0.4
  chore: release @cypress/react-v5.10.2
  chore: release @cypress/schematic-v1.5.3
  fix: remove outdated registry link (#18710)
  chore: release @cypress/schematic-v1.5.2
  chore: release create-cypress-tests-v1.1.3
  chore: Update Chrome (beta) to 96.0.4664.27 (#18676)
  chore(tests): Remove flaky assertion that relies on png how compression (#18668)
  fix: make sure to go back to no-specs when delete spec file (#17760)
  fix: Next.JS 12 components testing failing with ` TypeError: Cannot read property 'traceChild' of undefined` (#18648)
  Backport .gitignore from unified-desktop-gui
  chore(docs): add 'Upgrading Electron' instructions (#18594)
  release 8.7.0 [skip ci]
  ...
tgriesser added a commit that referenced this pull request Nov 3, 2021
* develop: (40 commits)
  fix(driver): Sticky elements within a fixed container will not prevent an element from being scrolled to (#18441)
  chore: make `create` function on server.ts obsolete (#18615)
  docs: Add instructions to squash commits to develop in Contributing (#18728)
  fix(@cypress/react): throw if using Next.js swc-loader without nodeVersion=system (#18686)
  refactor: remove Ramda (#18723)
  chore: Increase paralleled machines for desktop-gui tests (#18725)
  chore: Update Chrome (stable) to 95.0.4638.69 (#18696)
  chore: release @cypress/vue-v3.0.4
  chore: release @cypress/react-v5.10.2
  chore: release @cypress/schematic-v1.5.3
  fix: remove outdated registry link (#18710)
  chore: release @cypress/schematic-v1.5.2
  chore: release create-cypress-tests-v1.1.3
  chore: Update Chrome (beta) to 96.0.4664.27 (#18676)
  chore(tests): Remove flaky assertion that relies on png how compression (#18668)
  fix: make sure to go back to no-specs when delete spec file (#17760)
  fix: Next.JS 12 components testing failing with ` TypeError: Cannot read property 'traceChild' of undefined` (#18648)
  Backport .gitignore from unified-desktop-gui
  chore(docs): add 'Upgrading Electron' instructions (#18594)
  release 8.7.0 [skip ci]
  ...
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.

None yet

4 participants