Skip to content

AFTE RELEASE feat(customization): typed customization registry via AppTypes.Customizations#6133

Open
sedghi wants to merge 1 commit into
masterfrom
ohifTypeOHIFCustomization
Open

AFTE RELEASE feat(customization): typed customization registry via AppTypes.Customizations#6133
sedghi wants to merge 1 commit into
masterfrom
ohifTypeOHIFCustomization

Conversation

@sedghi

@sedghi sedghi commented Jul 10, 2026

Copy link
Copy Markdown
Member

Context

The CustomizationService exposes ~90 customization ids, but getCustomization(id) returns the loose Customization union for all of them. There is no discoverability of which ids exist or what shape each expects, and consumers compensate with ~20 as unknown as X / as any casts across the repo. setCustomizations payloads (including $set / $push command specs used by modes and app config) are not checked at all.

Changes

This PR adds the typing infrastructure (no per-key population yet, and no runtime changes):

  • AppTypes.Customizations registry — an empty, declaration-merged global interface in platform/core/src/types/AppTypes.ts, following the exact pattern already used by AppTypes.Services and PresentationIds. Each extension (in-tree or third-party) declares the ids it owns:

    declare global {
      namespace AppTypes {
        interface Customizations {
          'viewportOverlay.topLeft': OverlayItem[];
          'panelSegmentation.disableEditing': boolean;
        }
      }
    }
  • Typed overloads on getCustomization, getValue, and hasCustomization: registered ids get autocomplete and their declared return type; any other string (dynamic keys, undeclared third-party keys) falls back to the existing Customization | undefined signature, so nothing breaks.

  • Checked setCustomizations payloads via the new CustomizationEntries type: registered ids accept their declared value directly or as an immutability-helper spec ({ $set: ... }, { $push: [...] }, the custom $filter, ...). CustomizationPhaseInput reuses it, so phase-tagged appConfig.customizationService blocks written in TypeScript are checked the same way.

  • Two small call-site fixes: usAnnotation and basic-test-mode passed the raw string 'mode' where the CustomizationScope enum is expected (Mode is already the default scope, so the argument is dropped; runtime behavior identical).

  • CUSTOMIZATION_TYPING_PLAN.md documents the design, the conventions for declaring keys, and the follow-up phases (populating the registry per extension, then optionally generating the docs table and a JSON Schema for JSONC customization files from the registry).

Testing

  • All 68 CustomizationService unit tests pass.
  • Full-repo tsc diffed against a pre-change baseline: three pre-existing false positives fixed (valid customization objects rejected by the old Record<string, Customization> input type), zero new errors.
  • A standalone compile-time smoke test exercised the mechanism end to end (registry augmentation from an extension, typed reads, wrong-type reads rejected, $push of wrong element type rejected, $filter accepted, unknown keys and string module refs still compiling) under both the repo compiler settings and --strict.

Summary by CodeRabbit

  • New Features

    • Added discoverable, type-safe customization IDs with autocomplete and inferred value types.
    • Added validation for registered customization values and update commands.
    • Unknown or dynamic customization IDs remain supported through a flexible fallback.
    • Added typed support for filtering and updating customization entries.
  • Documentation

    • Added a rollout plan describing customization typing conventions, verification steps, and future registry expansion.

…stomizations

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.
@sedghi
sedghi temporarily deployed to fork-pr-approval July 10, 2026 16:22 — with GitHub Actions Inactive

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

Tip: disable this comment in your organization's Code Review settings.

@netlify

netlify Bot commented Jul 10, 2026

Copy link
Copy Markdown

Deploy Preview for ohif-dev failed. Why did it fail? →

Name Link
🔨 Latest commit 21ed32e
🔍 Latest deploy log https://app.netlify.com/projects/ohif-dev/deploys/6a511c3522d1910009b1ae69

@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR adds a declaration-merged customization registry, typed customization update entries, and registry-aware CustomizationService overloads. Two modes adopt the single-argument setCustomizations form, and a rollout document describes registration conventions and verification steps.

Changes

Typed customization API

Layer / File(s) Summary
Customization registry and entry types
platform/core/src/types/AppTypes.ts, platform/core/src/services/CustomizationService/types.ts, platform/core/src/services/CustomizationService/customizationUrlTypes.ts
Adds the AppTypes.Customizations registry, typed customization entries, $filter command shapes, and typed phase inputs.
CustomizationService typed overloads
platform/core/src/services/CustomizationService/CustomizationService.ts
Adds registry-keyed overloads for lookup and existence checks, updates getValue, and changes setCustomizations to accept CustomizationEntries.
Mode integration and rollout specification
modes/basic-test-mode/src/index.ts, modes/usAnnotation/src/index.ts, CUSTOMIZATION_TYPING_PLAN.md
Updates both modes to call setCustomizations with one argument and documents the customization typing rollout.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

  • OHIF/Viewers#5806: Adds and consumes segmentation customization ids with typed customization modules.
  • OHIF/Viewers#5992: Touches CustomizationService lookup and customization payload typing.

Suggested reviewers: jbocce

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title is specific and matches the main change, despite the stray 'AFTE RELEASE' prefix.
Description check ✅ Passed The description covers context, changes, and testing clearly, but it omits the checklist and tested-environment sections from the template.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ohifTypeOHIFCustomization

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cypress

cypress Bot commented Jul 10, 2026

Copy link
Copy Markdown

Viewers    Run #6515

Run Properties:  status check passed Passed #6515  •  git commit 21ed32ea49: feat(customization): add typed customization registry via AppTypes.Customization...
Project Viewers
Branch Review ohifTypeOHIFCustomization
Run status status check passed Passed #6515
Run duration 01m 40s
Commit git commit 21ed32ea49: feat(customization): add typed customization registry via AppTypes.Customization...
Committer Alireza
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 28
View all changes introduced in this branch ↗︎

@sedghi sedghi changed the title feat(customization): typed customization registry via AppTypes.Customizations AFTE RELEASE feat(customization): typed customization registry via AppTypes.Customizations Jul 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant