feat(dicomImageLoader): add wasmBasePath option for codec binaries - #2826
feat(dicomImageLoader): add wasmBasePath option for codec binaries#2826wayfarer3130 wants to merge 3 commits into
Conversation
Each decoder locates its WASM binary with a bare `@cornerstonejs/codec-...`
specifier inside `new URL(..., import.meta.url)`. Bundlers do not rewrite bare
specifiers there, so bundled applications get a request for a path that does
not exist - typically answered by the SPA fallback, surfacing as:
CompileError: WebAssembly.instantiate(): expected magic word 00 61 73 6d,
found 3c 21 64 6f (3c 21 64 6f is "<!do")
The only fix available to consumers today is to post-process the built worker
and rewrite those specifiers, which every bundler-based integration has to
reinvent and which breaks silently whenever the decoders change.
Add a single `wasmBasePath` loader option instead:
dicomImageLoaderInit({ wasmBasePath: '/assets/cs-wasm/' })
One root for all four codecs - deliberately no per-codec paths. The directory
holds the binaries under their published file names. A relative path resolves
against the decode worker's location; absolute paths and full URLs (e.g. a CDN)
are used as given. When unset, the existing import.meta.url resolution applies,
so unbundled and script-tag usage is unaffected.
The value reaches the decoders through the per-task decode config, and is
applied before any codec initializes.
Also documents the option and replaces the docs reference to
`setConfiguration({ wasmBasePath })`, which was never implemented.
There was a problem hiding this comment.
Claude Code Review
Claude Code Review is paused for this repository. To reconnect it, an admin of this repository's GitHub organization (or the account owner, for personal repositories) who can also manage your Claude organization's Code Review settings needs to re-link GitHub in Code Review settings. This is a one-time step.
Tip: disable this comment in your organization's Code Review settings.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds configurable codec WASM base-path support, forwards the option to decode workers, applies shared URL resolution across codec decoders, and documents deployment setup and required binary filenames. ChangesConfigurable codec WASM paths
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Loader as createImage
participant Worker as decodeImageFrame
participant Decoder as Codec initializer
participant Resolver as resolveWasmUrl
Loader->>Worker: pass wasmBasePath in decodeConfig
Worker->>Decoder: initialize decoder
Decoder->>Resolver: resolve codec WASM filename
Resolver-->>Decoder: return WASM URL
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/dicomImageLoader/src/decodeImageFrameWorker.js`:
- Around line 367-369: Update the decode-task setup around
setWasmBasePathFromConfig so each task explicitly resets the WASM base path when
decodeConfig does not provide one, while preserving configured paths when
present. Ensure a later unconfigured task falls back to the codec’s built-in URL
instead of inheriting state from an earlier task.
In `@packages/docs/docs/getting-started/vue-angular-react-vite.md`:
- Line 104: Update the diagnostic code fence in vue-angular-react-vite.md to
include an explicit language identifier, such as text, immediately after the
opening fence.
- Line 80: Update the Vue subpath setup bullet in vue-angular-react-vite.md to
include the same codec WASM guidance as the React entry: link to “Codec WASM
location” and mention configuring init({ wasmBasePath }) when serving codec
binaries yourself.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 93ec5169-8bd3-4c90-ab54-bd58108b2b18
📒 Files selected for processing (12)
packages/dicomImageLoader/src/__tests__/wasmBasePath.spec.tspackages/dicomImageLoader/src/decodeImageFrameWorker.jspackages/dicomImageLoader/src/imageLoader/createImage.tspackages/dicomImageLoader/src/shared/decoders/decodeHTJ2K.tspackages/dicomImageLoader/src/shared/decoders/decodeJPEG2000.tspackages/dicomImageLoader/src/shared/decoders/decodeJPEGBaseline8Bit.tspackages/dicomImageLoader/src/shared/decoders/decodeJPEGLS.tspackages/dicomImageLoader/src/shared/wasmBasePath.tspackages/dicomImageLoader/src/types/LoaderDecodeOptions.tspackages/dicomImageLoader/src/types/LoaderOptions.tspackages/dicomImageLoader/src/types/WebWorkerTypes.tspackages/docs/docs/getting-started/vue-angular-react-vite.md
A decodeConfig without wasmBasePath leaves the current value in place instead of clearing it, which reads like leaked state across decode tasks. It is load bearing: every decoder calls its own initialize()/initLibjpegTurbo() with no arguments from decodeAsync, so a reset on undefined would clear the path between the task setup and the codec's first locateFile call - reintroducing the bare specifier 404 the option exists to fix. Behavior and tests unchanged; the invariant was only documented on the setter.
The Codec WASM location section claimed bundlers do not rewrite bare specifiers inside new URL(). That is only true of esbuild. Verified against the pattern the decoders use: webpack 5.105.0 resolves it via the package exports map, emits the asset Vite 8.1.5 resolves it, emits the asset (inlines under the size limit) esbuild 0.24.2 passes it through unchanged esbuild 0.28.1 passes it through unchanged So Angular's esbuild-based application builder is the case that needs wasmBasePath, while the Vite templates work without it. State the per-bundler behavior, note that Vite's dev prebundling runs esbuild (which is what the existing optimizeDeps.exclude guidance is for), and give Vue/React a short pointer framed as optional - serving the binaries from a chosen location such as a CDN - rather than as required setup.
|
There was an earlier change that was much more extensive than this one: #1982 |
The DICOM image loader's decoders locate their WASM binaries with a bare
@cornerstonejs/codec-... specifier inside new URL(..., import.meta.url).
Angular's application builder is esbuild-based, and esbuild passes those
specifiers through unchanged (verified on 0.24.2 and 0.28.1; webpack 5 and Vite
both resolve them), so the request 404s, the SPA fallback HTML reaches the
decoder, and decoding dies with "expected magic word 00 61 73 6d, found 3c 21
64 6f".
scripts/bundle-dicom-worker.js used to rewrite those four specifiers inside the
bundled worker. Replace that with the loader's wasmBasePath option, a single
root for every codec:
dicomImageLoaderInit({ maxWebWorkers: 1, wasmBasePath })
It resolves against document.baseURI, so one build works at the site root and
under /subpath/. The codec binaries now land in one flat directory,
public/cs-dicom-loader/wasm/, matching the option's contract. The bundling
script keeps a build-time check that warns - rather than fails, so pnpm install
can still complete - when the installed loader has no wasmBasePath support.
wasmBasePath is not in a Cornerstone3D release yet; see
cornerstonejs/cornerstone3D#2826. scripts/link-cs3d.js points the app at a local
Cornerstone3D build until it ships (pnpm link:cs3d, link:cs3d:status,
unlink:cs3d). It copies each package's dist rather than symlinking: a symlinked
dist makes the bundler resolve the loader's own dependencies out of the
Cornerstone3D checkout, where this app's Node built-in stubs do not exist, and
the build fails with Could not resolve "fs".
Verified against the local build with the rewrite removed - dev, dev:subpath,
preview and preview:subpath each render the CT volume, 135/135 frames decoded,
codec wasm 200, no console errors.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Each decoder locates its WASM binary with a bare
@cornerstonejs/codec-...specifier insidenew URL(..., import.meta.url). Bundlers do not rewrite bare specifiers there, so bundled applications get a request for a path that does not exist - typically answered by the SPA fallback, surfacing as:CompileError: WebAssembly.instantiate(): expected magic word 00 61 73 6d,
found 3c 21 64 6f (3c 21 64 6f is "<!do")
The only fix available to consumers today is to post-process the built worker and rewrite those specifiers, which every bundler-based integration has to reinvent and which breaks silently whenever the decoders change.
Add a single
wasmBasePathloader option instead:dicomImageLoaderInit({ wasmBasePath: '/assets/cs-wasm/' })
One root for all four codecs - deliberately no per-codec paths. The directory holds the binaries under their published file names. A relative path resolves against the decode worker's location; absolute paths and full URLs (e.g. a CDN) are used as given. When unset, the existing import.meta.url resolution applies, so unbundled and script-tag usage is unaffected.
The value reaches the decoders through the per-task decode config, and is applied before any codec initializes.
Also documents the option and replaces the docs reference to
setConfiguration({ wasmBasePath }), which was never implemented.Summary by CodeRabbit
New Features
wasmBasePathto control where codec decoder WASM binaries are loaded from.Bug Fixes
Documentation
wasmBasePathconfiguration.Tests