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.
* bump version * bump beta (#2638) * bump beta [BUMP BETA MAJOR] (#2639) * bump version * bump version [BUMP BETA MAJOR] * chore(version): Update package versions to 5.0.0-beta.1 [skip ci] * Update to use feat/use-beta-5.0-cs3d for downstream test * Revert to earlier bun/node * fix: Dependencies * feat: Add metadata module to single-source metadata (#2625) * feat: Add metadata module to single-source metadata * Revert some of hte changes to dicom image loader in favour of deprecation * Undo change * Move unit tests to metadata package * Revert changes to adapters * Revert unneeded changes * Update metadata module to use AsyncDicomListener format * fix: Add unit test * Add metadata provider and init for the new metadata function * Split module lists * update metadata versoin * Move audit to end so build steps are viewable * fix: Audit last * fix lock file mismatch * fix: metadata reference * fix: Link to metadata * fix: Move audit to a single build to allow overall PR status Previously the audit happened early and multiple times, causing issues when the audit failures were unrelated to other changes. * fix: Unused components, add ecg module * Remove subdirectory import * fix: ecg module * Remove unused lower camel case reader * Switch new/old flag to be useLegacyMetadataProvider * Add metadata module for github builds * useLegacyMetadataProvider for the karma tests needing it * Add legacy module name * PR comments * fix: Wado URI test * Update version * Missing PR rename issue * Fix init/register options to allow registering new provider This explains the differences between the legacy/default metadata provider and the data loaders themselves, which still need registering. * Add rebuild:canvas command to fix missing canvas * fix: Update way that loadImage functions with naturalized data * Cleaner listener design/setup and pas (more) tests * fix: loadImage with metadata module * fix: Transfer syntax return as string * fix: Wrong load image was getting used * Update the register to clear data * Store worker log to loglevel * fix: Various karma tests * fix: Remaining karma issues * fix: Scaling data unit tests weren't running correctly * Use fixed page size to make tests run consistently. * Set size for consistency * docusaurus build issues * Build fixes * Version fix * Update to include metadata module * Include metadata module in circleci config * fix: Build when linked * fix: workspace dependency to allow yarn 1.x * Update package module for adapters build * Try to fix adapters build issue * Export suv scaling factor * Update bun version to push an update * Link suv module specifically * fix: Merge issues * bun lock * Make node directory if it doesn't exist * fix: Add migration guide and remove prebuild:esm * Rename natural to naturalizedMetadata * More renames natural => naturalized * PR review comments * Push test versoin * Run against master instead of beta * Rerun tests * fix: Allow lower ohif_ref for reference * Invalid image position for PT and some positions with parent position * Kick updated build * PR comments to use a separate pipeline for the add handling * fix: Aspect ratio - merge had gone bad and undo missed changes * fix: Unintended merge * Only remove imageId prefix if there are at least two schemes * PR review comment fixes * Create @corenrstonejs/utils package with initial contents * fix: install * Use load local instead of upload * Fix karma tests * fix: Build issues * fix: Types on toNumber * fix: ohif-downstream to use build:esm * toNumber * Force retest * Change to allow 120 minutes for CS3D playwright tests * Update retries to match OHIF * Upload playwright results on failure * fix: Add utils path * Rename addPart10Instance * Remove cache warm up in utils * Fix function rename * PR comments - use enum values instead of literals. * fix: Resolutions so that beta installs again * Remove bom * lock * Fix docusaurus and docs build warnings/errors * tmp vuln * cornerstone3d redo viewports (#2666) * feat: Update to use pnpm and remove bun/yarn usage (#2635) * feat: pnpm 11 build/install for cs3d * Format check runs at root level rather than in each package * Enable corepack detection of pnpm version * Fix audit issue * ohif-downstream build issue * Update to beta.3 for pnpm * Update beta 3 in pnpm lock * Re-run test on updated OHIF * Fix pnpm build based on merge commit * Fix link/unlik * Fixes for linking * fix cs3d link with ohif * ci: pin Node 24 and bump pnpm to 11.4.0 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> * chore: drop exact packageManager pin, rely on engines pnpm>=11 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> * chore: restore packageManager pnpm pin for nested-repo corepack 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> * fix(ohif-downstream): install OHIF with bun, not pnpm 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> * Update bun lock * frozen workfiles * Move frozen lockfile flag to pnpm-workspace.yaml * lock * Rerun build, resetting ohif-downstream to default * fix: unit test run * Fix chrome headless run for karma tests * Update tmp 0.2.6 to resolve the high severity issue --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Joe Boccanfuso <joe.boccanfuso@radicalimaging.com> * fix tests beta (#2749) * feat: Preserve originally-requested viewport type in rendering engines (#2751) * feat: Migrate to pnpm and update workflows for codemods package (#2752) * fix: resolve CI failures from audit and CodeQL alerts Bump vitest and @vitest/browser-playwright to 4.1.8 to clear the critical GHSA-2h32-95rg-cppp advisory blocking the CircleCI checkout job's security audit. Also address CodeQL findings introduced by the viewport refactor diff: switch example RNGs to crypto APIs, guard the annotation manager against prototype-polluting keys, and run rspack via spawnSync with an args array instead of a shell-interpolated string. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: add manual approval gate before npm publish Insert a HOLD_FOR_APPROVAL job between BUILD and NPM_PUBLISH in the TEST_AND_RELEASE workflow so the computed version can be reviewed and approved in the CircleCI UI before any release is published. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Rebuild to check overall build status --------- Co-authored-by: Alireza <ar.sedghi@gmail.com> Co-authored-by: ohif-bot <contact@ohif.org> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Joe Boccanfuso <joe.boccanfuso@radicalimaging.com>
* bump version * bump beta (#2638) * bump beta [BUMP BETA MAJOR] (#2639) * bump version * bump version [BUMP BETA MAJOR] * chore(version): Update package versions to 5.0.0-beta.1 [skip ci] * Update to use feat/use-beta-5.0-cs3d for downstream test * Revert to earlier bun/node * fix: Dependencies * feat: Add metadata module to single-source metadata (#2625) * feat: Add metadata module to single-source metadata * Revert some of hte changes to dicom image loader in favour of deprecation * Undo change * Move unit tests to metadata package * Revert changes to adapters * Revert unneeded changes * Update metadata module to use AsyncDicomListener format * fix: Add unit test * Add metadata provider and init for the new metadata function * Split module lists * update metadata versoin * Move audit to end so build steps are viewable * fix: Audit last * fix lock file mismatch * fix: metadata reference * fix: Link to metadata * fix: Move audit to a single build to allow overall PR status Previously the audit happened early and multiple times, causing issues when the audit failures were unrelated to other changes. * fix: Unused components, add ecg module * Remove subdirectory import * fix: ecg module * Remove unused lower camel case reader * Switch new/old flag to be useLegacyMetadataProvider * Add metadata module for github builds * useLegacyMetadataProvider for the karma tests needing it * Add legacy module name * PR comments * fix: Wado URI test * Update version * Missing PR rename issue * Fix init/register options to allow registering new provider This explains the differences between the legacy/default metadata provider and the data loaders themselves, which still need registering. * Add rebuild:canvas command to fix missing canvas * fix: Update way that loadImage functions with naturalized data * Cleaner listener design/setup and pas (more) tests * fix: loadImage with metadata module * fix: Transfer syntax return as string * fix: Wrong load image was getting used * Update the register to clear data * Store worker log to loglevel * fix: Various karma tests * fix: Remaining karma issues * fix: Scaling data unit tests weren't running correctly * Use fixed page size to make tests run consistently. * Set size for consistency * docusaurus build issues * Build fixes * Version fix * Update to include metadata module * Include metadata module in circleci config * fix: Build when linked * fix: workspace dependency to allow yarn 1.x * Update package module for adapters build * Try to fix adapters build issue * Export suv scaling factor * Update bun version to push an update * Link suv module specifically * fix: Merge issues * bun lock * Make node directory if it doesn't exist * fix: Add migration guide and remove prebuild:esm * Rename natural to naturalizedMetadata * More renames natural => naturalized * PR review comments * Push test versoin * Run against master instead of beta * Rerun tests * fix: Allow lower ohif_ref for reference * Invalid image position for PT and some positions with parent position * Kick updated build * PR comments to use a separate pipeline for the add handling * fix: Aspect ratio - merge had gone bad and undo missed changes * fix: Unintended merge * Only remove imageId prefix if there are at least two schemes * PR review comment fixes * Create @corenrstonejs/utils package with initial contents * fix: install * Use load local instead of upload * Fix karma tests * fix: Build issues * fix: Types on toNumber * fix: ohif-downstream to use build:esm * toNumber * Force retest * Change to allow 120 minutes for CS3D playwright tests * Update retries to match OHIF * Upload playwright results on failure * fix: Add utils path * Rename addPart10Instance * Remove cache warm up in utils * Fix function rename * PR comments - use enum values instead of literals. * fix: Resolutions so that beta installs again * Remove bom * lock * Fix docusaurus and docs build warnings/errors * tmp vuln * cornerstone3d redo viewports (#2666) * feat: Use new extensible model for registering viewports * feat: Update to use pnpm and remove bun/yarn usage (#2635) * feat: pnpm 11 build/install for cs3d * Format check runs at root level rather than in each package * Enable corepack detection of pnpm version * Fix audit issue * ohif-downstream build issue * Update to beta.3 for pnpm * Update beta 3 in pnpm lock * Re-run test on updated OHIF * Fix pnpm build based on merge commit * Fix link/unlik * Fixes for linking * fix cs3d link with ohif * ci: pin Node 24 and bump pnpm to 11.4.0 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> * chore: drop exact packageManager pin, rely on engines pnpm>=11 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> * chore: restore packageManager pnpm pin for nested-repo corepack 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> * fix(ohif-downstream): install OHIF with bun, not pnpm 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> * Update bun lock * frozen workfiles * Move frozen lockfile flag to pnpm-workspace.yaml * lock * Rerun build, resetting ohif-downstream to default * fix: unit test run * Fix chrome headless run for karma tests * Update tmp 0.2.6 to resolve the high severity issue --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Joe Boccanfuso <joe.boccanfuso@radicalimaging.com> * Remove bun/yarn locks * high vuln axios issue * fix tests beta (#2749) * feat: Preserve originally-requested viewport type in rendering engines (#2751) * feat: Migrate to pnpm and update workflows for codemods package (#2752) * fix: resolve CI failures from audit and CodeQL alerts Bump vitest and @vitest/browser-playwright to 4.1.8 to clear the critical GHSA-2h32-95rg-cppp advisory blocking the CircleCI checkout job's security audit. Also address CodeQL findings introduced by the viewport refactor diff: switch example RNGs to crypto APIs, guard the annotation manager against prototype-polluting keys, and run rspack via spawnSync with an args array instead of a shell-interpolated string. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: add manual approval gate before npm publish Insert a HOLD_FOR_APPROVAL job between BUILD and NPM_PUBLISH in the TEST_AND_RELEASE workflow so the computed version can be reviewed and approved in the CircleCI UI before any release is published. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: Use better example of extension viewport - contour3d * Rebuild to check overall build status --------- Co-authored-by: Alireza <ar.sedghi@gmail.com> Co-authored-by: ohif-bot <contact@ohif.org> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Joe Boccanfuso <joe.boccanfuso@radicalimaging.com>
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