-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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(core): fix missing import #23109
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
☁️ Nx Cloud ReportCI is running/has finished running commands for commit a38dd32. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 6 targets
Sent with 💌 from NxCloud. |
2cb96b3
to
997cbb0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey 👋 - performance
is available on Node 16 and higher, which means that its available for all of the node versions we currently target.
I'm going to accept this PR, but for future reference you may encounter other issues if using a lower node version. See: https://nx.dev/nx-api/workspace/documents/nx-nodejs-typescript-version-matrix
This was actually breaking with node v20. According to the doc, you need to do the import, see https://nodejs.org/api/perf_hooks.html |
I'm using node 20.12.0 and have this problem with the last nx version when I run jest test for my nx angular application plugin. |
Here's the docs that show perf was added as a global in node v16, if you get an error about it not being defined then there is something causing Nx to be ran with a different node version than you are expecting. https://nodejs.org/docs/latest/api/globals.html#performance You can test this locally in your terminal with |
Agree, and I've it in node, and I've only one instance of node, the version 20, but with the last version of nx, I got this problem while running jest.
But with this patch, it is working |
@AgentEnder @alaingiller I imagine this is a jest (Providing the snippet from Craigory |
Good point, I effectively have jest-environment-jsdom. |
@JamesHenry @AgentEnder. .The jest plugin uses jsdom under the hood : jsdom/jsdom#3309. Jsdom does not implement the
Maybe we can update the generator to use node for tests ( for plugin ) That way, you could remove all those |
This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request. |
@chabb Thanks that makes sense, I'll open an issue to track |
Current Behavior
The import for
performance
is missing, which causes an error to appearExpected Behavior
No error should occured
Related Issue(s)
Fixes #