Skip to content

Update to use beta 5.0 of CS3D#5904

Open
wayfarer3130 wants to merge 32 commits into
masterfrom
feat/use-beta-5.0-cs3d
Open

Update to use beta 5.0 of CS3D#5904
wayfarer3130 wants to merge 32 commits into
masterfrom
feat/use-beta-5.0-cs3d

Conversation

@wayfarer3130
Copy link
Copy Markdown
Contributor

@wayfarer3130 wayfarer3130 commented Mar 17, 2026

Context

Changes & Results

Testing

Checklist

PR

  • [] My Pull Request title is descriptive, accurate and follows the
    semantic-release format and guidelines.

Code

  • [] My code has been well-documented (function documentation, inline comments,
    etc.)

Public Documentation Updates

  • [] The documentation page has been updated as necessary for any public API
    additions or removals.

Tested Environment

  • [] OS:
  • [] Node version:
  • [] Browser:

Greptile Summary

This PR upgrades the codebase to CS3D beta 5.0, adjusting metadata provider registration order (providers must be added after initWADOImageLoader because the new linked-metadata branch clears them during loader init), fixing Jest module resolution for @cornerstonejs/calculate-suv, and tightening watch/symlink config for local CS3D development.

  • Metadata ordering fix (init.tsx): providers are now registered after initWADOImageLoader; however genericMetadataProvider.get is still passed without .bind() unlike the two adjacent providers.
  • New isEqual utility (platform/core/src/utils/isEqual.js): re-exports utilities.isEqual from @cornerstonejs/core, but @cornerstonejs/core is not declared in platform/core/package.json and the property is accessed at module evaluation time, so a missing or changed package silently exports undefined.
  • Dev tooling (rsbuild.config.ts, webpack.pwa.js): watch options are flipped to track @cornerstonejs symlinks; lazyCompilation: false is added unconditionally to rsbuild, affecting all developers.

Confidence Score: 3/5

Not ready to merge — the new isEqual.js utility introduces an undeclared @cornerstonejs/core dependency inside platform/core that will cause build/test failures, and the missing .bind() on genericMetadataProvider.get in init.tsx can silently break metadata retrieval at runtime.

Two independent defects are present on critical paths: the undeclared @cornerstonejs/core import in platform/core/src/utils/isEqual.js will produce a module-not-found error in any build or test that touches that module, and the missing .bind() in init.tsx can cause silent metadata-retrieval failures at runtime.

platform/core/src/utils/isEqual.js (undeclared dependency, property access at eval time) and extensions/cornerstone/src/init.tsx (missing .bind() on genericMetadataProvider.get).

Important Files Changed

Filename Overview
platform/core/src/utils/isEqual.js New utility re-exporting utilities.isEqual from @cornerstonejs/core; imports cornerstone in platform/core (architectural coupling) and evaluates the property at module load time, so the export silently becomes undefined if the property is absent.
extensions/cornerstone/src/init.tsx Metadata provider registration moved after initWADOImageLoader; genericMetadataProvider.get is added without .bind(), unlike the two adjacent providers — an existing flagged issue still present in this diff.
rsbuild.config.ts Adds lazyCompilation: false globally and flips watchOptions to follow @cornerstonejs symlinks; disabling lazy compilation affects all developers' dev-server startup time.
platform/app/.webpack/webpack.pwa.js Flips watchOptions.ignored to watch only @cornerstonejs and adds followSymlinks for local CS3D symlink development.
playwright.config.ts Reduces CI retries from 3→1, adds maxFailures:10, and switches reporter to always-HTML; consistent with the workflow artifact change.
.github/workflows/playwright.yml Updates artifact copy to use -r on the playwright-report directory instead of a single JSON file, matching the reporter change in playwright.config.ts.
extensions/cornerstone/jest.config.js Adds explicit calculate-suv mapping before the general @cornerstonejs pattern to handle its non-standard dist/ layout.
extensions/default/jest.config.js Same calculate-suv mapping fix as extensions/cornerstone/jest.config.js.
jest.config.base.js Moves @cornerstonejs module mappings to the base config so all packages inherit them, with calculate-suv handled before the general pattern.
extensions/default/src/getPTImageIdInstanceMetadata.ts Tightens import of InstanceMetadata/PhilipsPETPrivateGroup to type-only, eliminating a runtime value import of type-only symbols.
platform/core/src/utils/index.ts Exports new isEqual utility; inherits the dependency and property-access-at-eval concerns from isEqual.js.

Comments Outside Diff (2)

  1. tests/mpr2.spec.ts, line 3 (link)

    Missing visitStudyRendered import causes ReferenceError

    The import still pulls in visitStudy (which is no longer called), but visitStudyRendered — which is called in beforeEach — is never imported. This will throw ReferenceError: visitStudyRendered is not defined at runtime and fail every test in this file.

  2. tests/DicomTagBrowser.spec.ts, line 26 (link)

    Stale waitForVolumeLoad calls will fail

    waitForVolumeLoad was renamed to waitForViewportsRendered in MainToolbarPageObject.ts, but this file still calls the old name at lines 26, 43, 97, and 122. The PR only updated line 61 (one of five call sites). All four remaining calls will throw TypeError: mainToolbarPageObject.waitForVolumeLoad is not a function and fail every affected test.

  3. tests/mpr2.spec.ts, line 1 (link)

    visitStudyRendered called but never imported

    The import on line 1 still pulls in visitStudy (which is no longer called), but visitStudyRendered — called in beforeEach — is absent from the import. Every test in this file will fail with ReferenceError: visitStudyRendered is not defined at runtime.

  4. tests/DicomTagBrowser.spec.ts, line 26 (link)

    Stale waitForVolumeLoad calls remain at four sites

    waitForVolumeLoad was renamed to waitForViewportsRendered in MainToolbarPageObject.ts, but lines 26, 43, 97, and 122 of this file still call the old name. Only line 61 was updated in this PR. All four remaining call sites will throw TypeError: mainToolbarPageObject.waitForVolumeLoad is not a function and fail every affected test.

    Affected lines: 26, 43, 97, 122 — e.g. line 26:

  5. tests/DicomTagBrowser.spec.ts, line 26 (link)

    Stale waitForVolumeLoad calls at four remaining sites

    waitForVolumeLoad was renamed to waitForViewportsRendered in MainToolbarPageObject.ts, but this file still calls the old name at lines 26, 43, 97, and 122. Only line 61 was updated in this PR. All four remaining call sites will throw TypeError: mainToolbarPageObject.waitForVolumeLoad is not a function and fail every affected test.

  6. tests/mpr2.spec.ts, line 14 (link)

    Stale waitForVolumeLoad call

    waitForVolumeLoad was renamed to waitForViewportsRendered in MainToolbarPageObject.ts by this PR, but line 14 of this file still calls the old name. Every test in this file will throw TypeError: mainToolbarPageObject.waitForVolumeLoad is not a function.

  7. tests/DicomTagBrowser.spec.ts, line 26 (link)

    Stale waitForVolumeLoad calls remain unfixed

    waitForVolumeLoad was renamed to waitForViewportsRendered in MainToolbarPageObject.ts by this PR. Line 61 was updated, but lines 26, 43, 97, and 122 still call the old name and will throw TypeError: mainToolbarPageObject.waitForVolumeLoad is not a function, failing every affected test.

  8. extensions/default/src/init.ts, line 2 (link)

    @cornerstonejs/metadata not declared as a dependency

    @cornerstonejs/metadata is imported here for calculateSUVScalingFactors, but this package appears in neither package.json nor bun.lock. The same undeclared import is used in platform/core/src/classes/MetadataProvider.ts and platform/core/src/utils/imageIdToURI.js. Any build or test that touches these modules will fail with a module-not-found error until the package is added as a dependency and the lockfile is updated.

  9. platform/core/src/utils/imageIdToURI.js, line 3 (link)

    imageIdToURI may silently export undefined

    utilities.imageIdToURI is accessed as a property of the imported object at module evaluation time. If @cornerstonejs/metadata is not yet installed (it is absent from every package.json and bun.lock in this repo) or if a future version of that package moves the function off utilities, the default export resolves to undefined. Every caller that then invokes imageIdToURI(id) will throw TypeError: imageIdToURI is not a function — a silent, hard-to-diagnose failure compared to the previous self-contained implementation.

    The missing-dependency problem (@cornerstonejs/metadata not declared in platform/core/package.json or extensions/default/package.json) must be resolved first; without it the module cannot be loaded at all.

  10. platform/core/src/utils/imageIdToURI.js, line 1-3 (link)

    @cornerstonejs/metadata undeclared dependency

    @cornerstonejs/metadata is imported here (and in MetadataProvider.ts and extensions/default/src/init.ts), but it does not appear in any package.json in this repo, nor in bun.lock. Any build or test that touches these modules will fail with a module-not-found error. Additionally, utilities.imageIdToURI is accessed as a property at module evaluation time, so if the package is absent or moves the function off utilities, every call-site receives TypeError: imageIdToURI is not a function rather than a clear import error.

  11. extensions/default/src/init.ts, line 2 (link)

    calculateSUVScalingFactors moved to undeclared package

    calculateSUVScalingFactors is now imported from @cornerstonejs/metadata, which is not listed in extensions/default/package.json or any other package.json in the repo. While @cornerstonejs/calculate-suv (the previous source) remains a declared dependency, the runtime import will fail until @cornerstonejs/metadata is properly added to the dependency tree and lockfile.

  12. platform/core/src/utils/imageIdToURI.js, line 1-3 (link)

    @cornerstonejs/metadata undeclared in all affected packages

    @cornerstonejs/metadata is imported in three files (platform/core/src/utils/imageIdToURI.js, platform/core/src/classes/MetadataProvider.ts, and extensions/default/src/init.ts) but does not appear in any package.json in the repo (checked platform/core, extensions/default, and the root). Any build or test that resolves these modules will fail with a module-not-found error. Additionally, because imageIdToURI.js exports utilities.imageIdToURI as a property access at evaluation time, a missing or changed package causes every caller to receive TypeError: imageIdToURI is not a function rather than a clear import error.

  13. platform/core/src/logger.ts, line 1 (link)

    @cornerstonejs/utils undeclared in platform/core

    @cornerstonejs/utils is imported here but does not appear in platform/core/package.json, the root package.json, or either lock file (bun.lock / yarn.lock). Any build or test that resolves this module will fail with a module-not-found error. Please add @cornerstonejs/utils to platform/core/package.json (and run the installer to update the lock file) before merging.

  14. extensions/cornerstone/src/init.tsx, line 194 (link)

    genericMetadataProvider.get passed without .bind()

    The two other providers on lines 195–200 both use .bind() to preserve the correct this context when metaData calls the function. genericMetadataProvider.get is passed as a bare method reference, so if it accesses this internally it will receive undefined (strict mode) or the global object, causing silent data-retrieval failures at runtime.

  15. platform/core/src/utils/toNumber.js, line 1-4 (link)

    utilities.toNumber is accessed as a property of the imported namespace at module evaluation time. If @cornerstonejs/metadata is absent (it is not declared in any package.json in this repo) or if a future version moves toNumber off utilities, the default export silently becomes undefined. Every call-site then throws TypeError: toNumber is not a function instead of a clear import error — the same fragile pattern already flagged for imageIdToURI.js. The undeclared dependency must be added to package.json before this is safe to use, and a named import would give a build-time error rather than a silent undefined.

  16. extensions/cornerstone/src/init.tsx, line 194 (link)

    The two adjacent providers both use .bind() to preserve the correct this context when metaData invokes the callback. genericMetadataProvider.get is passed as a bare method reference, so if the method reads this internally it will receive undefined in strict mode or the global object, causing silent metadata-retrieval failures at runtime.

  17. platform/core/src/utils/isEqual.js, line 2 (link)

    @cornerstonejs/utils not declared in platform/core

    @cornerstonejs/utils is imported here but is absent from platform/core/package.json (neither in dependencies nor peerDependencies). This is the same undeclared-dependency pattern already flagged for platform/core/src/logger.ts. Any build or test that resolves this module will fail with a module-not-found error until @cornerstonejs/utils is added to platform/core/package.json and the lockfile is updated.

  18. platform/core/src/utils/isEqual.js, line 5-7 (link)

    @cornerstonejs/core introduced as a dependency of platform/core

    @cornerstonejs/core is not listed in platform/core/package.json, so this import will fail at build or test time with a module-not-found error. Beyond the missing declaration, placing a Cornerstone dependency inside platform/core couples the framework-agnostic core to a specific rendering library — the JSDoc comment in this very file acknowledges this pattern should be avoided ("Prefer importing utilities from @cornerstonejs/core in extension code"). Additionally, utilities.isEqual is accessed as a property at module evaluation time: if the property is absent on the exported object the default export silently becomes undefined, and every caller throws TypeError: isEqual is not a function rather than a clear import error.

Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
platform/core/src/utils/isEqual.js:5-7
**`@cornerstonejs/core` introduced as a dependency of `platform/core`**

`@cornerstonejs/core` is not listed in `platform/core/package.json`, so this import will fail at build or test time with a module-not-found error. Beyond the missing declaration, placing a Cornerstone dependency inside `platform/core` couples the framework-agnostic core to a specific rendering library — the JSDoc comment in this very file acknowledges this pattern should be avoided ("Prefer importing `utilities` from `@cornerstonejs/core` in extension code"). Additionally, `utilities.isEqual` is accessed as a property at module evaluation time: if the property is absent on the exported object the default export silently becomes `undefined`, and every caller throws `TypeError: isEqual is not a function` rather than a clear import error.

### Issue 2 of 2
rsbuild.config.ts:34-36
**`lazyCompilation: false` applied globally, not gated to CS3D local dev**

The surrounding comment explains the `watchOptions` change is for symlinked local CS3D development, but `lazyCompilation: false` is unconditional and affects every developer running the dev server. With lazy compilation disabled, rsbuild compiles all modules upfront instead of on demand, which can significantly increase cold-start time. Consider gating it on the same condition used for `followSymlinks`, e.g. only when a `CS3D_LOCAL` or similar env var is set.

Reviews (21): Last reviewed commit: "Fixing build issues" | Re-trigger Greptile

@netlify
Copy link
Copy Markdown

netlify Bot commented Mar 17, 2026

Deploy Preview for ohif-dev ready!

Name Link
🔨 Latest commit 67650ce
🔍 Latest deploy log https://app.netlify.com/projects/ohif-dev/deploys/6a079c653346c000087e9e51
😎 Deploy Preview https://deploy-preview-5904--ohif-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@wayfarer3130 wayfarer3130 added the ohif-integration Causes a CS3D/OHIF integraiton build to be run label Mar 17, 2026
@cypress
Copy link
Copy Markdown

cypress Bot commented Mar 17, 2026

Viewers    Run #6267

Run Properties:  status check passed Passed #6267  •  git commit 67650ce53a: Fixing build issues
Project Viewers
Branch Review feat/use-beta-5.0-cs3d
Run status status check passed Passed #6267
Run duration 02m 16s
Commit git commit 67650ce53a: Fixing build issues
Committer Bill Wallace
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 37
View all changes introduced in this branch ↗︎

@wayfarer3130 wayfarer3130 changed the base branch from master to beta April 23, 2026 12:10
@wayfarer3130 wayfarer3130 changed the title [WIP] Update to use beta 5.0 of CS3D Update to use beta 5.0 of CS3D Apr 23, 2026
@wayfarer3130 wayfarer3130 changed the base branch from beta to master April 30, 2026 19:35
@wayfarer3130 wayfarer3130 requested a review from sedghi May 15, 2026 22:33
@wayfarer3130
Copy link
Copy Markdown
Contributor Author

The PR: cornerstonejs/cornerstone3D#2735 demonstrates that this branch work successfully with origin/beta as well as this version (well, origin/beta as it will be as soon as the next merge from origin/main passes)


initWADOImageLoader(userAuthenticationService, appConfig, extensionManager);

// Add OHIF metadata providers after dicomImageLoader.init().
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This works fine against origin/main as well as against beta, but it is reuqired for beta.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ohif-integration Causes a CS3D/OHIF integraiton build to be run

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants