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(vite): PCV3 Plugin update to use resolveConfig #21287

Merged
merged 1 commit into from
Jan 26, 2024

Conversation

ndcunningham
Copy link
Contributor

No description provided.

@ndcunningham ndcunningham self-assigned this Jan 23, 2024
Copy link

vercel bot commented Jan 23, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Updated (UTC)
nx-dev ⬜️ Ignored (Inspect) Visit Preview Jan 25, 2024 8:08pm

@ndcunningham ndcunningham force-pushed the fix/vite-use-resolve-config branch from 941346a to 1e99127 Compare January 23, 2024 21:18
@ndcunningham ndcunningham marked this pull request as ready for review January 23, 2024 21:19
@ndcunningham ndcunningham requested a review from a team as a code owner January 23, 2024 21:19
@ndcunningham ndcunningham force-pushed the fix/vite-use-resolve-config branch from 1e99127 to 7a448a2 Compare January 24, 2024 17:02
@FrozenPandaz
Copy link
Collaborator

Victor got this error when running with vite in CI:

~/p/t/p/test (main|✔) $ rm -rf node_modules/ pnpm-lock.yaml && CI=true pnpm i
Progress: resolved 1, reused 0, downloaded 0, added 0
Progress: resolved 33, reused 33, downloaded 0, added 0
Progress: resolved 89, reused 81, downloaded 0, added 0
Progress: resolved 317, reused 299, downloaded 0, added 0
Progress: resolved 701, reused 649, downloaded 1, added 0
Progress: resolved 1012, reused 960, downloaded 1, added 0
 WARN  2 deprecated subdependencies found: [email protected], [email protected]
Packages: +967
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Progress: resolved 1017, reused 966, downloaded 1, added 967, done
.../[email protected]/node_modules/esbuild postinstall$ node install.js
.../[email protected]/node_modules/esbuild postinstall: Done
.../[email protected]/node_modules/cypress postinstall$ node index.js --exec install
.../[email protected]/node_modules/cypress postinstall: Cypress 13.6.3 is installed in /home/jason/.cache/Cypress/13.6.3
.../[email protected]/node_modules/cypress postinstall: Done
.../node_modules/nx postinstall$ node ./bin/post-install
.../node_modules/nx postinstall: The CJS build of Vite's Node API is deprecated. See https://vitejs.dev/guide/troubleshooting.html#vite-cjs-node-api-deprecated for more details.
.../node_modules/nx postinstall: (node:46908) [DEP0147] DeprecationWarning: In future versions of Node.js, fs.rmdir(path, { recursive: true }) will be removed. Use fs.rm(path, { recursive: true }) instead
.../node_modules/nx postinstall: (Use `node --trace-deprecation ...` to show where the warning was created)
.../node_modules/nx postinstall: ✘ [ERROR] Could not resolve "/home/jason/projects/triage/pcv3/test/node_modules/.pnpm/[email protected]_@[email protected]_@[email protected]/node_modules/nx/apps/app1/vite.config.ts"
.../node_modules/nx postinstall: failed to load config from /home/jason/projects/triage/pcv3/test/node_modules/.pnpm/[email protected]_@[email protected]_@[email protected]/node_modules/nx/apps/app1/vite.config.ts
.../node_modules/nx postinstall: Unable to create nodes for apps/app1/vite.config.ts using plugin @nx/vite/plugin.
.../node_modules/nx postinstall: 	 Inner Error: Error: Build failed with 1 error:
.../node_modules/nx postinstall: error: Could not resolve "/home/jason/projects/triage/pcv3/test/node_modules/.pnpm/[email protected]_@[email protected]_@[email protected]/node_modules/nx/apps/app1/vite.config.ts"
.../node_modules/nx postinstall:     at failureErrorWithLog (/home/jason/projects/triage/pcv3/test/node_modules/.pnpm/[email protected]/node_modules/esbuild/lib/main.js:1651:15)
.../node_modules/nx postinstall:     at /home/jason/projects/triage/pcv3/test/node_modules/.pnpm/[email protected]/node_modules/esbuild/lib/main.js:1059:25
.../node_modules/nx postinstall:     at runOnEndCallbacks (/home/jason/projects/triage/pcv3/test/node_modules/.pnpm/[email protected]/node_modules/esbuild/lib/main.js:1486:45)
.../node_modules/nx postinstall:     at buildResponseToResult (/home/jason/projects/triage/pcv3/test/node_modules/.pnpm/[email protected]/node_modules/esbuild/lib/main.js:1057:7)
.../node_modules/nx postinstall:     at /home/jason/projects/triage/pcv3/test/node_modules/.pnpm/[email protected]/node_modules/esbuild/lib/main.js:1086:16
.../node_modules/nx postinstall:     at responseCallbacks.<computed> (/home/jason/projects/triage/pcv3/test/node_modules/.pnpm/[email protected]/node_modules/esbuild/lib/main.js:704:9)
.../node_modules/nx postinstall:     at handleIncomingPacket (/home/jason/projects/triage/pcv3/test/node_modules/.pnpm/[email protected]/node_modules/esbuild/lib/main.js:764:9)
.../node_modules/nx postinstall:     at Socket.readFromStdout (/home/jason/projects/triage/pcv3/test/node_modules/.pnpm/[email protected]/node_modules/esbuild/lib/main.js:680:7)
.../node_modules/nx postinstall:     at Socket.emit (node:events:514:28)
.../node_modules/nx postinstall:     at addChunk (node:internal/streams/readable:545:12)
.../node_modules/nx postinstall:     at readableAddChunkPushByteMode (node:internal/streams/readable:495:3)
.../node_modules/nx postinstall:     at Socket.Readable.push (node:internal/streams/readable:375:5)
.../node_modules/nx postinstall:     at Pipe.onStreamRead (node:internal/stream_base_commons:190:23)
.../node_modules/nx postinstall: Done

We can fix it by passing in the absolute path of the viteConfig. Can we sneak that into this PR please? 🙏

@ndcunningham ndcunningham force-pushed the fix/vite-use-resolve-config branch from 7a448a2 to d785db0 Compare January 24, 2024 18:02
@ndcunningham ndcunningham added scope: core core nx functionality scope: plugins and removed scope: core core nx functionality labels Jan 24, 2024
@jaysoo
Copy link
Member

jaysoo commented Jan 24, 2024

It's still not working for this repo https://github.com/jaysoo/vite-app-libs

I think once the config is resolved we still need to check the inputs to make sure they exist?

@ndcunningham ndcunningham force-pushed the fix/vite-use-resolve-config branch from d785db0 to 6d877dd Compare January 24, 2024 22:56
@ndcunningham
Copy link
Contributor Author

It's still not working for this repo https://github.com/jaysoo/vite-app-libs

I think once the config is resolved we still need to check the inputs to make sure they exist?

There was a problem using resolveConfig looking through the vite code it merges other configs. So library configs would contain build after the resolve. Which is not expected?

Maybe we can look into that further, But moving forward, using loadConfigFromFile respects the config so we can determine if the config is buildable.

@ndcunningham ndcunningham force-pushed the fix/vite-use-resolve-config branch from 6d877dd to a4ff6ab Compare January 25, 2024 00:34
Copy link
Member

@jaysoo jaysoo left a comment

Choose a reason for hiding this comment

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

Qwik's config loads as

  path: '/Users/jack/projects/qwik-app/vite.config.ts',
  config: {
    plugins: [ [Array], [Object], [Object] ],
    server: { headers: [Object] },
    preview: { headers: [Object] }
  },
  dependencies: [ 'vite.config.ts' ]
}

We should get this to work because unlike Remix, Qwik expects users to run vite build, etc.

Example: https://app.warp.dev/block/xuT3vrlbiAmWrnHvoQK7TF

@ndcunningham ndcunningham force-pushed the fix/vite-use-resolve-config branch from a4ff6ab to b7dd016 Compare January 25, 2024 17:12
@ndcunningham ndcunningham force-pushed the fix/vite-use-resolve-config branch from b7dd016 to 6e0c284 Compare January 25, 2024 20:07
@ndcunningham ndcunningham requested a review from jaysoo January 25, 2024 20:39
@jaysoo jaysoo merged commit bf6f78f into nrwl:master Jan 26, 2024
6 checks passed
@ndcunningham ndcunningham deleted the fix/vite-use-resolve-config branch January 30, 2024 17:13
Copy link

github-actions bot commented Feb 5, 2024

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants