Skip to content

Commit 21ed32e

Browse files
committed
feat(customization): add typed customization registry via AppTypes.Customizations
Seeds a declaration-merged AppTypes.Customizations registry so extensions can declare the value type of each customization id they own, and adds typed overloads to getCustomization, getValue, hasCustomization and setCustomizations. Registered ids get autocomplete and precise types; unregistered and dynamic ids keep the existing loose Customization fallback, so nothing breaks. setCustomizations payloads (including immutability-helper command specs and the custom $filter) are checked against the registry through the new CustomizationEntries type, which CustomizationPhaseInput now reuses for phased app-config blocks. Also drops a redundant string 'mode' scope argument in two modes that was mistyped against the CustomizationScope enum (Mode is the default). CUSTOMIZATION_TYPING_PLAN.md documents the design and the follow-up phases for populating the registry across extensions.
1 parent f79055f commit 21ed32e

7 files changed

Lines changed: 328 additions & 111 deletions

File tree

CUSTOMIZATION_TYPING_PLAN.md

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
# Typed Customizations: Design and Rollout Plan
2+
3+
This document accompanies the PR that introduces the `AppTypes.Customizations`
4+
registry and describes the follow-up work needed to make every customization id
5+
in OHIF fully typed and discoverable.
6+
7+
## Problem
8+
9+
`customizationService.getCustomization(id)` accepts any string and returns the
10+
`Customization` union, which is broad enough to be effectively untyped. As a
11+
result:
12+
13+
- There is no way to discover which customization ids exist from the code; the
14+
docs table (`platform/docs/docs/platform/services/customization-service/sampleCustomizations.tsx`)
15+
is hand-maintained and already drifts from the registered keys.
16+
- Consumers cast at the call site (`as unknown as ColorbarCustomization`,
17+
`as string`, `as any`, ...) — roughly 20 such casts exist across the repo.
18+
- `setCustomizations` payloads (including `$set` / `$push` / `$merge` command
19+
specs used by modes and app config) are not checked at all.
20+
21+
## Design (implemented in this PR)
22+
23+
A declaration-merged registry, following the same pattern already used for
24+
`AppTypes.Services` and `PresentationIds`:
25+
26+
1. `platform/core/src/types/AppTypes.ts` seeds an empty global interface:
27+
28+
```ts
29+
declare global {
30+
namespace AppTypes {
31+
export interface Customizations {}
32+
}
33+
}
34+
```
35+
36+
2. Each extension (in-tree or third-party) merges the keys it owns:
37+
38+
```ts
39+
declare global {
40+
namespace AppTypes {
41+
interface Customizations {
42+
'viewportOverlay.topLeft': OverlayItem[];
43+
'panelSegmentation.disableEditing': boolean;
44+
}
45+
}
46+
}
47+
```
48+
49+
3. The service API is overloaded so registered ids get autocomplete and precise
50+
types, while unregistered ids (dynamic keys such as
51+
`` `${buttonSectionId}.config` ``, third-party keys that have not been
52+
declared) keep working through a plain-string fallback:
53+
54+
- `getCustomization('viewportOverlay.topLeft')` returns `OverlayItem[]`.
55+
- `getCustomization('some.dynamic.key')` returns `Customization | undefined`
56+
exactly as before.
57+
- `setCustomizations({...})` checks registered ids against their declared
58+
value type, either as a direct value or as an immutability-helper spec
59+
(`{ $set: ... }`, `{ $push: [...] }`, the custom `$filter`, ...), via the
60+
`CustomizationEntries` type in
61+
`platform/core/src/services/CustomizationService/types.ts`.
62+
- `CustomizationPhaseInput` (the `bootstrap` / `global` / `mode` blocks of
63+
`appConfig.customizationService`) reuses `CustomizationEntries`, so
64+
phase-tagged config written in TypeScript is checked the same way.
65+
66+
Nothing about the runtime changes; this is purely additive typing.
67+
68+
### Conventions for declaring keys
69+
70+
- The extension that registers a key's default owns its type declaration.
71+
Export the value type from the `*Customization.ts` file that produces the
72+
default, and add the registry entry in the extension's `types/AppTypes.ts`.
73+
- Declare a key with `| undefined` when no default is shipped for it, so strict
74+
consumers must handle its absence. (Note: the repo's own tsconfig does not
75+
enable `strictNullChecks`, so this protects downstream strict consumers.)
76+
- Declared types describe the resolved value after `inheritsFrom` /
77+
`transform`, i.e. what `getCustomization` actually returns.
78+
79+
## Next steps
80+
81+
### Phase 2: populate the registry for extension-default and extension-cornerstone
82+
83+
The bulk of the value: these two extensions register roughly 75 of the ~90
84+
known ids. For each key:
85+
86+
1. Export its value type from the producer file (most already have local
87+
interfaces or obvious shapes).
88+
2. Add the entry to the extension's `types/AppTypes.ts` augmentation.
89+
3. Remove the now-redundant casts at consumer call sites; each removed cast is
90+
a free correctness check.
91+
92+
Suggested PR split: one PR per extension.
93+
94+
### Phase 3: remaining extensions and the core-owned keys
95+
96+
- `cornerstone-dicom-seg` (`segmentation.store.*`, `segmentation.segmentLabel`,
97+
`cornerstone.segmentation.loadMultiframeAsPart10`), `cornerstone-dicom-sr`
98+
(`onBeforeSRAddMeasurement`, `onBeforeDicomStore`, `codingValues`, ...),
99+
`cornerstone-dicom-rt`, `cornerstone-dicom-pmap`, `dicom-microscopy`,
100+
`measurement-tracking` (`measurement.prompt*`, `viewportNotification.*`),
101+
`cornerstone-dynamic-volume`.
102+
- Keys consumed by `platform/core` / `platform/ui-next` but defaulted in
103+
`extension-default` (`instanceSortingCriteria`, `sortingCriteria`,
104+
`studyBrowser.sortFunctions`) must be declared in `platform/core` itself so
105+
the dependency direction stays core -> extension-free; `extension-default`
106+
imports those types from core.
107+
- Modes' `setCustomizations` calls in `onModeEnter` become checked
108+
automatically once the keys they touch are declared.
109+
110+
### Phase 4 (optional follow-ups)
111+
112+
- Generate the docs customization table from the registry instead of the
113+
hand-maintained `sampleCustomizations.tsx`, so docs cannot drift.
114+
- Generate a JSON Schema from `AppTypes.Customizations` to give editor
115+
IntelliSense and validation for the JSONC `?customization=` files and the
116+
`customizationService` section of config files.
117+
- Consider a lint rule nudging away from `getValue`-with-cast toward the typed
118+
`getCustomization` overload.
119+
120+
## Verification recipe used for this PR
121+
122+
- `pnpm --filter @ohif/core exec jest src/services/CustomizationService` — all
123+
68 unit tests pass.
124+
- Full-repo `npx tsc --noEmit --emitDeclarationOnly false -p tsconfig.json`
125+
diffed against a pre-change baseline: three pre-existing false positives
126+
fixed, zero new errors. (The repo has ~5.7k pre-existing tsc errors and no
127+
tsc CI gate; diffing sorted error lists is the reliable check.)
128+
- A standalone compile-time smoke test exercised the mechanism end to end
129+
(registry augmentation, typed reads, spec checking, string fallback) under
130+
both the repo's compiler settings and `--strict`.

modes/basic-test-mode/src/index.ts

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -180,21 +180,18 @@ function modeFactory() {
180180
'WindowLevelRegion',
181181
]);
182182

183-
customizationService.setCustomizations(
184-
{
185-
'ohif.hotkeyBindings': {
186-
$push: [
187-
{
188-
commandName: 'undo',
189-
label: 'Undo',
190-
keys: ['ctrl+z'],
191-
isEditable: true,
192-
},
193-
],
194-
},
183+
customizationService.setCustomizations({
184+
'ohif.hotkeyBindings': {
185+
$push: [
186+
{
187+
commandName: 'undo',
188+
label: 'Undo',
189+
keys: ['ctrl+z'],
190+
isEditable: true,
191+
},
192+
],
195193
},
196-
'mode'
197-
);
194+
});
198195
},
199196
onModeExit: ({ servicesManager }: withAppTypes) => {
200197
const {

modes/usAnnotation/src/index.ts

Lines changed: 80 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -178,93 +178,90 @@ function modeFactory({ modeConfiguration }) {
178178
'WindowLevelRegion',
179179
]);
180180

181-
customizationService.setCustomizations(
182-
{
183-
'panelSegmentation.disableEditing': {
184-
$set: true,
185-
},
186-
autoCineModalities: {
187-
$set: [],
188-
},
189-
'ohif.hotkeyBindings': {
190-
$push: [
191-
{
192-
commandName: 'switchUSAnnotationToPleuraLine',
193-
label: 'Add new pleura line',
194-
keys: ['W'],
195-
},
196-
{
197-
commandName: 'switchUSAnnotationToBLine',
198-
label: 'Add new B-line',
199-
keys: ['S'],
200-
},
201-
{
202-
commandName: 'deleteLastPleuraAnnotation',
203-
label: 'Delete last pleura line',
204-
keys: ['E'],
205-
},
206-
{
207-
commandName: 'deleteLastBLineAnnotation',
208-
label: 'Delete last B-line',
209-
keys: ['D'],
210-
},
211-
{
212-
commandName: 'toggleDisplayFanAnnotation',
213-
label: 'Toggle overlay',
214-
keys: ['O'],
215-
},
216-
],
217-
},
218-
measurementsContextMenu: {
219-
inheritsFrom: 'ohif.contextMenu',
220-
menus: [
221-
// Get the items from the UI Customization for the menu name (and have a custom name)
222-
{
223-
id: 'forExistingMeasurement',
224-
selector: ({ nearbyToolData }) => !!nearbyToolData,
225-
items: [
226-
{
227-
label: 'Delete annotation',
228-
commands: 'removeMeasurement',
229-
},
230-
],
231-
},
232-
],
233-
},
234-
'viewportOverlay.topLeft': [
181+
customizationService.setCustomizations({
182+
'panelSegmentation.disableEditing': {
183+
$set: true,
184+
},
185+
autoCineModalities: {
186+
$set: [],
187+
},
188+
'ohif.hotkeyBindings': {
189+
$push: [
235190
{
236-
id: 'BLinePleuraPercentage',
237-
inheritsFrom: 'ohif.overlayItem',
238-
label: '',
239-
title: 'BLinePleuraPercentage',
240-
condition: ({ referenceInstance }) =>
241-
referenceInstance?.Modality.includes('US') && showPercentage,
242-
contentF: () => {
243-
const { viewportGridService, toolGroupService, cornerstoneViewportService } =
244-
servicesManager.services;
245-
const activeViewportId = viewportGridService.getActiveViewportId();
246-
const toolGroup = toolGroupService.getToolGroupForViewport(activeViewportId);
247-
if (!toolGroup) {
248-
return 'B-Line/Pleura : N/A';
249-
}
250-
const usAnnotation = toolGroup.getToolInstance(UltrasoundPleuraBLineTool.toolName);
251-
if (usAnnotation) {
252-
const viewport =
253-
cornerstoneViewportService.getCornerstoneViewport(activeViewportId);
254-
const percentage = usAnnotation.calculateBLinePleuraPercentage(viewport);
255-
if (percentage !== undefined) {
256-
return `B-Line/Pleura : ${percentage.toFixed(2)} %`;
257-
} else {
258-
return 'B-Line/Pleura : N/A';
259-
}
260-
}
261-
return 'B-Line/Pleura : N/A';
262-
},
191+
commandName: 'switchUSAnnotationToPleuraLine',
192+
label: 'Add new pleura line',
193+
keys: ['W'],
194+
},
195+
{
196+
commandName: 'switchUSAnnotationToBLine',
197+
label: 'Add new B-line',
198+
keys: ['S'],
199+
},
200+
{
201+
commandName: 'deleteLastPleuraAnnotation',
202+
label: 'Delete last pleura line',
203+
keys: ['E'],
204+
},
205+
{
206+
commandName: 'deleteLastBLineAnnotation',
207+
label: 'Delete last B-line',
208+
keys: ['D'],
209+
},
210+
{
211+
commandName: 'toggleDisplayFanAnnotation',
212+
label: 'Toggle overlay',
213+
keys: ['O'],
263214
},
264215
],
265216
},
266-
'mode'
267-
);
217+
measurementsContextMenu: {
218+
inheritsFrom: 'ohif.contextMenu',
219+
menus: [
220+
// Get the items from the UI Customization for the menu name (and have a custom name)
221+
{
222+
id: 'forExistingMeasurement',
223+
selector: ({ nearbyToolData }) => !!nearbyToolData,
224+
items: [
225+
{
226+
label: 'Delete annotation',
227+
commands: 'removeMeasurement',
228+
},
229+
],
230+
},
231+
],
232+
},
233+
'viewportOverlay.topLeft': [
234+
{
235+
id: 'BLinePleuraPercentage',
236+
inheritsFrom: 'ohif.overlayItem',
237+
label: '',
238+
title: 'BLinePleuraPercentage',
239+
condition: ({ referenceInstance }) =>
240+
referenceInstance?.Modality.includes('US') && showPercentage,
241+
contentF: () => {
242+
const { viewportGridService, toolGroupService, cornerstoneViewportService } =
243+
servicesManager.services;
244+
const activeViewportId = viewportGridService.getActiveViewportId();
245+
const toolGroup = toolGroupService.getToolGroupForViewport(activeViewportId);
246+
if (!toolGroup) {
247+
return 'B-Line/Pleura : N/A';
248+
}
249+
const usAnnotation = toolGroup.getToolInstance(UltrasoundPleuraBLineTool.toolName);
250+
if (usAnnotation) {
251+
const viewport =
252+
cornerstoneViewportService.getCornerstoneViewport(activeViewportId);
253+
const percentage = usAnnotation.calculateBLinePleuraPercentage(viewport);
254+
if (percentage !== undefined) {
255+
return `B-Line/Pleura : ${percentage.toFixed(2)} %`;
256+
} else {
257+
return 'B-Line/Pleura : N/A';
258+
}
259+
}
260+
return 'B-Line/Pleura : N/A';
261+
},
262+
},
263+
],
264+
});
268265
},
269266
onModeExit: ({ servicesManager }: withAppTypes) => {
270267
appConfig.disableConfirmationPrompts = settingsSaved.disableConfirmationPrompts;

0 commit comments

Comments
 (0)