fix the turbopack and jest bundling of proteinpaint code - #1763
Conversation
…de outside the project root dir
There was a problem hiding this comment.
Pull request overview
Re-enables local development against a sibling proteinpaint/client repo by adding a Turbopack config fragment that aliases @sjcrh/proteinpaint-client to a local build and widens Turbopack’s root so files outside the Next.js project can be bundled.
Changes:
- Added
ppTurbopackDevhelper to generate a Turbopack config fragment whenPP_CLIENT_DISTis set. - Updated
dev.shto drive the new workflow (nonpm link/ manualcp -r distintonode_modules). - Wired the helper into
packages/portal-proto/next.config.jsunderturbopack.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/portal-proto/src/features/proteinpaint/ppTurbopackDev.js | Adds Turbopack fragment to alias proteinpaint client to a local dist build and widen Turbopack root. |
| packages/portal-proto/src/features/proteinpaint/dev.sh | Updates local dev script to set PP_CLIENT_DIST and remove the old npm-link/copy workflow. |
| packages/portal-proto/next.config.js | Applies the optional proteinpaint Turbopack overrides during dev when PP_CLIENT_DIST is set. |
Suppressed comments (2)
packages/portal-proto/src/features/proteinpaint/dev.sh:27
- The comments describe
PP_CLIENT_DISTas pointing to a "dist" directory, but the script sets it to the client repo root (../proteinpaint/client). This mismatch makes it easy to set the variable incorrectly and end up with a confusingdist/dist/app.jsresolution in the Turbopack config.
# 2. PP_CLIENT_DIST (below) tells Turbopack to bundle the local client dist
# directly (see next.config.js `turbopack.resolveAlias`), so no npm link or
# manual `cp -r dist ...` into node_modules is needed. A browser refresh after
# a client rebuild is usually enough to pick up changes.
packages/portal-proto/src/features/proteinpaint/ppTurbopackDev.js:46
turbopack.rootis set to the common ancestor of the project and the client. When the two directories only share the filesystem root (e.g./), this widens Turbopack's root to the entire drive, which can significantly increase file-watching scope and slow down dev builds. Consider failing fast with a clear error when the computed ancestor is the filesystem root, so developers know to place the repos under a closer common parent.
return {
root: commonAncestor(projectDir, clientDir),
resolveAlias: { "@sjcrh/proteinpaint-client": rel },
};
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…red proteinpaint bundling
…inked mode and when set as a relative by the user
…nfig.ts, and consolidate all pp-specific next config overrides into a JSON-string env option
…es is an empty object in next.config.js
|
The changes to NextJS might not be necessary: Can you test this both in |
Description
This PR re-enables the dynamic rebundlng of proteinpaint client dist code when using the
portal-proto/src/features/proteinpaint/dev.shscript, which was previously broken after recent version bumps to the nextjs dependency. ProteinPaint code outside of the gff directory can now be dynamically bundled by turbopack and jest.@craigrbarnes Please verify that your
npm run devis not affected by these changes, I assume you don't have a local proteinpaint repo setup.Checklist
flagged issues
Screenshots/Screen Recordings (if Appropriate)