microsoft / microsoft/vscode-documentdb

Optional improvement: unify Fluent theme observation on the shared color store

Open
#922 0 comments 0 reactions 0 assignees View on GitHub

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 useActiveVSCodeTheme is 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.
  • useActiveVSCodeThemeKind remains standalone and preserves its public behavior.
  • useActiveVSCodeTheme still 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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.