chore(dockview-core): module boundary hardening (removability test + contract extraction)#1324
Merged
Merged
Conversation
Add a removability test: with any single module filtered out of the registered set, the core component must still construct, run the base layout operations (add / split / maximize / serialize / remove) and dispose without throwing. This is the contract that keeps module boundaries honest — if removing a module breaks core, the boundary is leaking. It immediately caught one: RootDropTargetModule was hard-wired in the constructor (this._rootDropTargetService.onWillShowOverlay/onDrop) and in updateOptions, so removing it crashed. Make the service access optional like every other module's — guard the edge-drop wiring behind its presence. Adds a small internal seam to register a module subset (defaults to the full set; not part of the public options surface). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…om impl) Relocate the service + host interfaces for ContextMenu, TabGroupChips and Accessibility out of their implementation files into a single core `moduleContracts.ts`. Core (the ServiceCollection slots in modules.ts and the host implementation in dockviewComponent.ts) now references the contracts from core, never from the implementation files. Each implementation imports its own contract from core. Pure type relocation — no behaviour change, all 1117 tests green. This makes the module boundary explicit: the contract is core, the implementation is a detail that can be relocated without core depending on it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mathuo
changed the title
Relocate IAdvancedDnDHost / IAdvancedDnDService into the core moduleContracts file, like the other three modules. Core now imports zero types from any of the four module implementation files — it references only the contracts — so each module is fully decoupled from core. The service already encapsulates its whole surface (preview overlay, group drag ghost, overlay-model resolution, onWill* dispatch), so only the contract moved. No behaviour change; 1117 tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Remove an unnecessary qualifier from the accessibility / live-region module doc comments; wording only, no code change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ry-hardening # Conflicts: # packages/dockview-core/src/dockview/accessibilityService.ts
|
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.
Two pieces of module-boundary hardening that keep the registered-module system honest and the seams explicit.
1. Removability contract test
With any single module filtered out of the registered set, the core component must still construct, run the base layout ops (add / split / maximize / serialize / remove) and dispose without throwing. Runs once per module + an all-present baseline.
It immediately caught a leak:
RootDropTargetModulewas hard-wired (non-?.) in the constructor andupdateOptions, so removing it crashed. Made the access optional like every other module's and guarded the edge-drop wiring behind the module's presence. Adds a small internal seam to register a module subset (defaults to the full set; not part of the public options surface).2. Contract extraction
Moved the service + host interfaces for ContextMenu, TabGroupChips and Accessibility out of their implementation files into a single core
moduleContracts.ts. Core (theServiceCollectionslots + the host implementation) now references contracts from core, never from the implementation files. Pure type relocation, no behaviour change.Together these make the module boundary explicit and provably clean: contracts are core, implementations are details that can be relocated without core depending on them.
Tests
1117 total (+12 removability). All green.