nrwl / nx

Smart Monorepos · Fast CI
https://nx.dev
MIT License
23k stars 2.3k forks source link

Unable to run load nextjs jest config outside of `nx run` #22450

Closed karlismelderis-mckinsey closed 6 days ago

karlismelderis-mckinsey commented 5 months ago

Current Behavior

seems that this PR broke simple script:

pnpx knip --config ./knip.js

with latest changes code fails here - https://github.com/nrwl/nx/blob/master/packages/next/plugins/with-nx.ts#L144 because none of these ENV variables are there

Also running jest tests outside of nx (e.g. IntelliJ) is not possible now

everything fails with same error:

  TypeError: Cannot read properties of undefined (reading 'data')
    at readConfigFileAndSetRootDir (/.../node_modules/.pnpm/jest-config@29.7.0_@types+node@20.11.30_ts-node@10.9.2/node_modules/jest-config/build/readConfigFileAndSetRootDir.js:116:13)

maybe with-nx.ts need to add 'phase-test' === phase here but I'm not familiar enough with NextJS plugin

Expected Behavior

I don't need to run every single script within nx run ...

GitHub Repo

No response

Steps to Reproduce

try to run knip of jest outside of nx

Nx Report

TypeError: Cannot read properties of undefined (reading 'data')
    at readConfigFileAndSetRootDir (/.../node_modules/.pnpm/jest-config@29.7.0_@types+node@20.11.30_ts-node@10.9.2/node_modules/jest-config/build/readConfigFileAndSetRootDir.js:116:13)


### Failure Logs

_No response_

### Package Manager Version

_No response_

### Operating System

- [X] macOS
- [ ] Linux
- [ ] Windows
- [ ] Other (Please specify)

### Additional Information

_No response_
karlismelderis-mckinsey commented 5 months ago

@jaysoo would you be able to check this bug?

karlismelderis-mckinsey commented 4 months ago

@FrozenPandaz and @ndcunningham how soon will you be able to pick this up?

if you're overloaded I can try to investigate internals of nx and come up with a proposal PR

ndcunningham commented 3 months ago

@karlismelderis-mckinsey feel free to pick this up if you can

johndalvik commented 1 month ago

@karlismelderis-mckinsey @ndcunningham

Any update on this?

I have to provide additional code to fix the behaviour, which is not not very clean and straightforward.

process.env['NX_TASK_TARGET_PROJECT'] = 'app';
process.env['NX_TASK_TARGET_TARGET'] = 'test';
process.env['NX_TASK_TARGET_CONFIGURATION'] = 'development';
mircoservices commented 1 week ago

Was able to fix the issue with the following patch

diff --git a/node_modules/@nx/next/plugins/with-nx.js b/node_modules/@nx/next/plugins/with-nx.js
index 5ba13ff..4e00bde 100644
--- a/node_modules/@nx/next/plugins/with-nx.js
+++ b/node_modules/@nx/next/plugins/with-nx.js
@@ -67,7 +67,7 @@ function withNx(_nextConfig = {}, context = getWithNxContext()) {
         // 2. During graph creation (i.e. create nodes), we won't have a graph to read, and it is not needed anyway since it's a build-time concern.
         //
         // NOTE: Avoid any `require(...)` or `import(...)` statements here. Development dependencies are not available at production runtime.
-        if (PHASE_PRODUCTION_SERVER === phase || global.NX_GRAPH_CREATION) {
+        if (PHASE_PRODUCTION_SERVER === phase || global.NX_GRAPH_CREATION || !process.env.NX_TASK_TARGET_TARGET) {
             const { nx, ...validNextConfig } = _nextConfig;
             return {
                 distDir: '.next',
ndcunningham commented 1 week ago

I don't think I see an issue with including TASK_TARGET_TARGET env var as we did before. We probably should've just added the graph creation instead of replacing it.