fix: Update to use pnpm and remove bun/yarn usage#2635
Conversation
|
Basically all I did was ask to remove lerna and switch to pnpm/node 24 for CS3D - this is more a straw man to talk about than a final version. Certainly all the tests/setup need to still be fixed |
|
@wayfarer3130 great start, could you do a sample with OHIF too |
|
@sedghi - passes the tests now and has nx removed |
|
Looks good i tested this. Can you do the same on OHIF so that we can merge together? Also don't see how we are versioning now |
|
Can't figure out how to get this working. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
# Conflicts: # .github/workflows/playwright.yml # .husky/pre-commit # package.json # packages/core/package.json
|
OHIF_REF: ohifohifupdateplaywightimgaes |
playwright.yml's matrix had regressed to Node 20 during the beta merge, but pnpm 11 requires Node >=22.13 (uses node:sqlite), so installs crashed with ERR_UNKNOWN_BUILTIN_MODULE. Restore Node 24 everywhere and move pnpm 11.1.1 -> 11.4.0 across all workflows and packageManager. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The packageManager: pnpm@11.4.0 pin forced pnpm's precise-version auto-switch, which broke commits for anyone whose local pnpm couldn't fetch that exact build. engines.pnpm ">=11" is sufficient; CI still pins 11.4.0 via pnpm/action-setup. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CS3D lives inside the OHIF repo, whose package.json pins packageManager: yarn. With corepack enabled, dropping CS3D's own packageManager field makes corepack walk up and resolve yarn for CS3D, breaking the pnpm-based husky hook. Pin pnpm here so corepack uses (and auto-fetches) pnpm for this workspace. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
OHIF is a bun/yarn project checked out inside this pnpm workspace. Installing it with pnpm walks up to this repo's pnpm-workspace.yaml (node-linker: hoisted) and produces a mixed node_modules where OHIF's jest and CS3D's jest copies get tangled, so jest-runner resolves to a different instance than @jest/core -> "Cannot read properties of undefined (reading 'leakDetector')". Install/run the OHIF side with bun (its native package manager) so it gets a self-contained node_modules. Adds a Set up Bun step; CS3D's own install/build stay on pnpm. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@sedghi - this PR now passes with the same OHIF downstream tests which were failing previously. Should we merge the pnpm side of this and then finish up the OHIF fixes? |
|
Sure lets do it |
jbocce
left a comment
There was a problem hiding this comment.
I am excited about this change - definitely tightens things up security wise especially for developers. That said, there are a couple of security items to consider. Thanks.
OHIF_REF: ohifohifupdateplaywightimgaes
Context
Switch to remove yarn/bun and use pnpm
Changes & Results
Testing
Checklist
PR
semantic-release format and guidelines.
Code
etc.)
Public Documentation Updates
additions or removals.
Tested Environment