feat: CLI command and environment variables (to support fhircast) - #6143
Open
awatson1978 wants to merge 18 commits into
Open
feat: CLI command and environment variables (to support fhircast) #6143awatson1978 wants to merge 18 commits into
awatson1978 wants to merge 18 commits into
Conversation
…lient id plumbing Platform enablers for FHIRcast v3.0.0 / SMART on FHIR integrations that live in out-of-tree plugins (e.g. github.com/node-on-fhir/nof-ohif-viewer): - EXTRA_EXTENSIONS / EXTRA_MODES env vars append plugins to pluginConfig at build time, including out-of-tree packages via name=directory entries, so deployments can add plugins without editing tracked files - /fhir-proxy dev-server proxy (FHIR_SERVER env var, WebSocket-enabled for FHIRcast subscriptions); env-configured PROXY_* entries now prepend to the proxy list instead of replacing it - SMART_CLIENT_ID added to the DefinePlugin whitelist with a documented .env placeholder - ModeRoute guards against the active data source not yet being registered when extension dependencies finish loading (SMART launch flows) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The chore(version) release-bot commits rewrote internal @ohif/* deps from workspace:* to concrete 3.13.0-beta.92 across the workspace package.json files but never regenerated the lockfile, so the lockfile specifiers still read workspace:*. CI's default --frozen-lockfile install aborted with ERR_PNPM_OUTDATED_LOCKFILE. Regenerate so specifiers match the manifests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…IR extension SMART_CLIENT_ID was a build-time DefinePlugin value and FHIR_SERVER / the /fhir-proxy dev-server route were FHIR-specific hooks that don't belong in upstream OHIF. Build-time bake-in also can't be changed without a rebuild, which is wrong for a deployment-specific client ID. These now live in the ohif-fhir-viewer extension as runtime config (window.config / Preferences / URL params), with the dev proxy replaced by server-side CORS. PR OHIF#5988 now carries only the generic core improvements: EXTRA_EXTENSIONS / EXTRA_MODES injection and the Mode.tsx late-dataSource guard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tension When an EXTRA_EXTENSIONS package contains a `mode/` subdirectory with a package.json, register that mode automatically — so `EXTRA_EXTENSIONS=<ext>` alone pulls in the extension and its bundled mode, with no separate EXTRA_MODES entry. An explicit EXTRA_MODES still wins (a mode already present by name is left untouched), and extensions without a `mode/` are unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds a third composition layer to the plugin-import generator so a build can
pin exactly which extensions/modes it ships in its own tracked file, without
editing the shared default pluginConfig.json. Keeps EXTRA_EXTENSIONS /
EXTRA_MODES unchanged (they still append last).
Layers, each overriding the previous:
1. pluginConfig.json static default
2. APP_PLUGIN_CONFIG=<file> tracked override; may `include` the default
(string or array), merges over it — plain
arrays append (de-duped by package name),
immutability-helper commands like { $set: [] }
replace a list. Same notation the
CustomizationService uses.
3. EXTRA_EXTENSIONS / EXTRA_MODES env name-injection, appended last.
Implemented entirely in writePluginImportsFile.js; all downstream resolution,
aliasing and asset copying already operate on the in-memory config, so they
inherit the merged result unchanged. Includes a cycle guard and fail-fast
errors. Per-mode dependency override left as a documented TODO(scope-B).
Adds pluginConfig.example.json (generic example) and documents the env vars
in platform/app/.env.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ension The ohif-fhir-viewer extension declares jszip and dcmjs-ecg as runtime dependencies, but pnpm-lock.yaml predated them. With frozenLockfile: true, a plain `pnpm install` refused to add them, so webpack failed to resolve both modules. Regenerate the lockfile so a frozen install pulls them in. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Re-resolve the extensions/ohif-fhir-viewer importer after switching its @ohif/* peerDependencies from published semver ranges to workspace:*. This prunes the published @ohif/*@3.12.5 + @cornerstonejs/*@4.15.29 subtree that the extension was pulling in (the broken nested @cornerstonejs/core that failed the webpack build) and links @ohif/core, extension-cornerstone, extension-default, and i18n to the in-tree workspace packages. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…d extensions The bundled-mode auto-detect previously scanned only EXTRA_EXTENSIONS, so an extension registered via pluginConfig.json (the ohif-cli link-extension path) loaded without its companion mode. Scan the fully-merged pluginConfig.extensions list instead, and route the EXTRA_EXTENSIONS/EXTRA_MODES layer through mergePluginList() so a plugin declared in both places registers once (env wins). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
✅ Deploy Preview for ohif-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
The merge commit resolved code conflicts but committed the lockfile with its git conflict markers still in place. The CI auto-merge of those markers left an overrides snapshot that no longer matches pnpm-workspace.yaml, failing the frozen install (ERR_PNPM_LOCKFILE_CONFIG_MISMATCH). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughPlugin composition documentation and example configuration were added, the FHIR viewer extension was registered, and mode initialization now waits for an available data source before continuing. ChangesPlugin composition and mode initialization
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
platform/app/src/routes/Mode/Mode.tsx (2)
130-155: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMake
ModeRoutereact to active data-source changes.dataSourceis read straight fromextensionManager, so the[dataSource]dependency only helps if some other render happens. Subscribe toExtensionManager.EVENTS.ACTIVE_DATA_SOURCE_CHANGEDhere (asDataSourceWrapperdoes), or move the value into React state/useSyncExternalStore, so late-registered sources can actually trigger initialization.🤖 Prompt for 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. In `@platform/app/src/routes/Mode/Mode.tsx` around lines 130 - 155, Update ModeRoute’s data-source handling so changes to extensionManager’s active data source trigger a React render and rerun the initialization effect. Subscribe to ExtensionManager.EVENTS.ACTIVE_DATA_SOURCE_CHANGED, following the existing DataSourceWrapper pattern, or expose the value through React state/useSyncExternalStore; preserve the current dataSource guard and initialization flow.
143-155: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGuard
setStudyInstanceUIDsagainst staleinitialize()completions.
WhendataSourcechanges, an older in-flightinitialize()can still resolve and overwrite the newer study list, which drives the viewer with stale data. Add a generation/cancel guard before setting state.🤖 Prompt for 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. In `@platform/app/src/routes/Mode/Mode.tsx` around lines 143 - 155, Update the initializeDataSource effect around dataSource.initialize so asynchronous completions are invalidated during cleanup or when dependencies change. Track an active generation/cancellation guard and check it before setStudyInstanceUIDs, ensuring only the latest initializeDataSource call can update state while preserving layoutTemplateData cleanup.
🤖 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.
Outside diff comments:
In `@platform/app/src/routes/Mode/Mode.tsx`:
- Around line 130-155: Update ModeRoute’s data-source handling so changes to
extensionManager’s active data source trigger a React render and rerun the
initialization effect. Subscribe to
ExtensionManager.EVENTS.ACTIVE_DATA_SOURCE_CHANGED, following the existing
DataSourceWrapper pattern, or expose the value through React
state/useSyncExternalStore; preserve the current dataSource guard and
initialization flow.
- Around line 143-155: Update the initializeDataSource effect around
dataSource.initialize so asynchronous completions are invalidated during cleanup
or when dependencies change. Track an active generation/cancellation guard and
check it before setStudyInstanceUIDs, ensuring only the latest
initializeDataSource call can update state while preserving layoutTemplateData
cleanup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d3423d86-fa4c-464a-a216-a3f810e1865e
⛔ Files ignored due to path filters (2)
platform/app/.webpack/writePluginImportsFile.jsis excluded by!**/.webpack/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
platform/app/.envplatform/app/pluginConfig.example.jsonplatform/app/pluginConfig.jsonplatform/app/src/routes/Mode/Mode.tsx
The master-merge commit absorbed a local ohif-cli link-extension registration into the tracked pluginConfig.json. CI clones do not contain extensions/ohif-fhir-viewer (it is a separate repository), so the generated pluginImports.js referenced an unresolvable package and the production build failed. The extension is deployment-time configuration: inject it via EXTRA_EXTENSIONS or APP_PLUGIN_CONFIG, or re-run the CLI locally. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
awatson1978
added a commit
to node-on-fhir/ohif-fhir-viewer
that referenced
this pull request
Jul 14, 2026
Point at the cli-tool PR branch, show both installation styles (EXTRA_EXTENSIONS env var and ohif-cli link-extension), document the companion-mode auto-detection, and warn against committing the link-extension pluginConfig.json entry (CI clones lack this extension). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Externally developed extensions (for example node-on-fhir/ohif-fhir-viewer, which adds FHIR data sources and SMART-on-FHIR launch toward FHIRcast support) currently require editing tracked files (
pluginConfig.json, webpack config) to be loaded by the viewer. That makes hosted/containerized deployments awkward — the plugin set can't vary per environment without forking tracked files — and keeps downstream forks in permanent conflict with upstream at the same lines.This PR makes plugin registration composable in three layers, so an extension can be injected at build time via environment variables (12-factor style), via a tracked override config, or via the classic
pluginConfig.json/ohif-clipath — and all three converge on the same mechanism inwritePluginImportsFile.js.Changes & Results
EXTRA_EXTENSIONS/EXTRA_MODESenv vars — inject plugins by package name (comma-separated, optional=<directory>override for out-of-tree checkouts) as the last configuration layer. No tracked-file edits needed to load an in-tree or out-of-tree extension.APP_PLUGIN_CONFIG— selects a tracked override plugin-config file that canincludethe default and extend/replace it (supports immutability-helper command specs like$set/$push). AddedpluginConfig.example.jsonas a template.mode/subdirectory (with its ownpackage.json) gets that mode registered automatically. Applies to any declared extension the build can locate on disk: in-tree (extensions/) or declared with adirectory— viapluginConfig.json,APP_PLUGIN_CONFIG, orEXTRA_EXTENSIONS. (An out-of-tree extension wired up withohif-cli link-extensionstill pairs withlink-mode, exactly as the CLI intends.)mergePluginList()(de-dupe by package name, env entry wins), so declaring the same plugin in both a config file and an env var registers it once.ModeRoutedata-source race fix (platform/app/src/routes/Mode/Mode.tsx) — a data source registered by an extension (e.g. entered via a SMART-on-FHIR launch) may not be active yet on the render where extension dependencies finish loading; bail out and re-run once it appears instead of crashing..envdocuments the new variables;pnpm-lock.yamlregenerated.Stock behavior is unchanged: with no env vars and the default
pluginConfig.json, the generatedpluginImports.jsis identical (no first-party extension bundles amode/).Testing
Clone the example extension out of tree (sibling of Viewers) and install each side:
CLI style (works today; verified end-to-end against master with the pnpm-compatible CLI):
Env-var style (this PR): one variable replaces both link commands and leaves the working tree untouched — the bundled mode is auto-detected:
In each case, visit
http://localhost:3000/fhir-viewer— the extension and its bundledfhir-viewermode both load. A stockpnpm dev(no env vars, default pluginConfig) is unaffected.Checklist
PR
semantic-release format and guidelines.
Code
etc.)
Public Documentation Updates
additions or removals. (The new env vars are documented in
platform/app/.env; happy to add a docs.ohif.org page if maintainers want one.)Tested Environment