microsoft / microsoft/vscode-documentdb
Optional improvement: unify Fluent theme observation on the shared color store
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 31
- Forks
- 22
- Avg merge
- 2d 20h
- Merged PRs (30d)
- 21
Description
Summary
Consider making the shared VS Code theme color store the single source for both the active theme kind and the generated Fluent theme.
This is optional architecture work. It is not required to fix the current stale-brand defect.
Source of the issue
PR #895 fixed a real same-kind refresh gap in useActiveVSCodeTheme. The hook previously regenerated the Fluent theme only when data-vscode-theme-kind changed. Most adapted Fluent tokens are live var(--vscode-*) expressions, but the brand ramp is a fixed hexadecimal snapshot generated from --vscode-button-background. Switching between two themes of the same kind, or changing workbench.colorCustomizations, could therefore leave the brand ramp stale.
The immediate fix subscribes useActiveVSCodeTheme to the existing themeColorsStore, which already observes root style and body theme-attribute changes for Monaco. useActiveVSCodeThemeKind still owns a separate MutationObserver for the body theme-kind attribute. This means the Fluent React path now has two observation mechanisms.
Proposed future option
Refactor the shared store to expose a stable snapshot containing both its version and the current theme kind, for example:
interface VSCodeThemeSnapshot {
readonly version: number;
readonly themeKind: string;
}
Both useActiveVSCodeThemeKind and useActiveVSCodeTheme could consume that snapshot through useSyncExternalStore. This would provide one observer, one subscription lifecycle, and one ordering model for root color changes and body theme metadata.
Potential benefits
- One coherent invalidation path for all React theme consumers.
- No duplicate body attribute observers when
useActiveVSCodeThemeis used. - Theme kind and color version come from the same external-store snapshot.
- Future snapshot-derived theme APIs can reuse the same lifecycle.
Costs and risks
- This is a broader public-hook refactor than the current defect requires.
- It changes lifecycle and identity assumptions for
useActiveVSCodeThemeKind. - It needs focused tests for first subscription, same-kind changes, kind changes, cleanup, multiple subscribers, and referential stability.
- The current extra observer may be effectively free in practice, so the architectural concern may be a false alarm.
Review requirement
Review this proposal in detail before implementation. Confirm through profiling or concrete lifecycle evidence that duplicate observation creates meaningful complexity or cost. If the current observers are cheap, correctly ordered, and easy to maintain, close this issue without code changes.
Acceptance criteria if pursued
- One shared external-store snapshot drives both theme kind and color invalidation.
useActiveVSCodeThemeKindremains standalone and preserves its public behavior.useActiveVSCodeThemestill regenerates fixed brand tokens on same-kind color changes.- Equal theme data preserves object identity.
- Multiple subscribers share one observer and cleanup occurs after the final unsubscribe.
- Existing Monaco theme behavior and first-subscription invalidation remain unchanged.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Review themeColorsStore, useActiveVSCodeThemeKind, and useActiveVSCodeTheme first, then profile or trace their observer lifecycles and subscription ordering. Decide whether duplicate observation creates meaningful cost or complexity; if pursued, focused tests should cover subscription, same-kind and kind changes, cleanup, multiple subscribers, identity, and existing Monaco behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- react, typescript
- Domain
- frontend
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100