Skip to content

refactor: introduce THEMES env var - #4642

Draft
mxschll wants to merge 6 commits into
mainfrom
dev-v3-schomax-theming
Draft

refactor: introduce THEMES env var#4642
mxschll wants to merge 6 commits into
mainfrom
dev-v3-schomax-theming

Conversation

@mxschll

@mxschll mxschll commented Jun 17, 2026

Copy link
Copy Markdown
Member

Description

Allows dynamic inclusion of themes using THEMES=visual-refresh,one-theme npm run build. This will populate INCLUDED_THEMES which we then can use in dev pages. Introduces theme= url param.

Note: We can remove the visualRefresh=true param in as a next step.

Related links, issue #, if available: n/a

How has this been tested?

Review checklist

The following items are to be evaluated by the author(s) and the reviewer(s).

Correctness

  • Changes include appropriate documentation updates.
  • Changes are backward-compatible if not indicated, see CONTRIBUTING.md.
  • Changes do not include unsupported browser features, see CONTRIBUTING.md.
  • Changes were manually tested for accessibility, see accessibility guidelines.

Security

Testing

  • Changes are covered with new/existing unit tests?
  • Changes are covered with new/existing integration tests?

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@mxschll mxschll changed the title refactor: introduce INCLUDED_THEMES env var refactor: introduce THEMES env var Jun 17, 2026
@codecov

codecov Bot commented Jun 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 53.92157% with 47 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.52%. Comparing base (f066dd8) to head (2e5af45).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
build-tools/utils/themes.js 53.92% 40 Missing and 7 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4642      +/-   ##
==========================================
- Coverage   97.66%   97.52%   -0.15%     
==========================================
  Files         958      960       +2     
  Lines       31317    31429     +112     
  Branches    11566    11603      +37     
==========================================
+ Hits        30585    30650      +65     
- Misses        686      726      +40     
- Partials       46       53       +7     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Replace the classic-primary-always/one-theme-boolean-secondary build with a
theme registry (build-tools/utils/themes.js) mapping theme ids to their
primary and/or secondary style-dictionary modules, resolved via two env vars:

- PRIMARY_THEME (classic|visual-refresh|core), default classic
- SECONDARY_THEMES (comma list of visual-refresh|one-theme), default
  visual-refresh

This lets a single build produce any of the four shipped compositions:
open-source (visual-refresh only), legacy console (classic+visual-refresh,
the unchanged default), console (visual-refresh+one-theme, no classic), and
core (core only). Unknown ids, ids used in a role they don't support, the
same id in both roles, or a duplicate secondary id all fail the build loudly
instead of silently degrading. Setting only one of the two env vars while a
different composition is already persisted now logs an explicit note that
the other one was inherited, not defaulted, before any validation error.

The resolved composition is "sticky": a bare quick-build inherits whatever
composition the last full build actually produced (read from
lib/components/internal/environment.json, before `clean` deletes it -- see
the load-order comment in themes.js), rather than resetting to the release
default. THEME_PRESET=dev (set only by package.json's start:watch script)
selects an all-themes preset for local dev without affecting the release
path; explicit env vars always win over both persisted and preset. `gulp
watch` refuses to silently adopt a different composition than what's already
built, since it never runs `clean` and would otherwise half-rebuild lib/.

Also:
- generate-environment.js now emits PRIMARY_THEME alongside INCLUDED_THEMES;
  ALWAYS_VISUAL_REFRESH is derived from the resolved primary theme.
- Adds style-dictionary/core/metadata.ts (core reused every other
  visual-refresh module already but never had its own metadata, so it could
  not be built as a primary theme at all until now).
- gulpfile.js's watch task now also reruns generateEnvironment so
  INCLUDED_THEMES/PRIMARY_THEME never go stale relative to what `styles` is
  producing.
- design-tokens.test.ts derives its theme list from the resolved build
  instead of a hardcoded ['classic', 'visual-refresh'].
- docs/SETUP.md documents the new env vars and the sticky-composition
  behavior.
- themes-persistence.test.js (a real end-to-end regression test that spawns
  gulp quick-build twice and mutates lib/) is excluded from the default
  test:unit run since it's ~60s and mutates shared build output; run it
  explicitly per the comment in jest.build-tools.config.js.

No-arg `npm run quick-build` output verified byte-identical to the
pre-change baseline (lib/components, lib/design-tokens), aside from the new
PRIMARY_THEME field in environment.{json,js,d.ts}.
Root cause: gulpfile.js's watch pipeline reran generateEnvironment/styles on
any src/**/*.scss change, including src/internal/generated/custom-css-properties/index.scss,
which every quick-build rewrites unconditionally (regardless of composition).
A gulp watch session left running from an earlier `npm start` would react to
a completely unrelated, concurrent quick-build's own generated-file write and
silently reassert its own (now stale) captured composition over the newer
build's output -- reproduced end-to-end and confirmed as the cause of the
"one-theme disappeared" / "classic came back" reports.

Two fixes:
- gulpfile.js: exclude src/internal/generated/custom-css-properties/** (and
  the other task-generated paths: src/index.ts, src/icon/generated/**) from
  the watch globs -- nothing in the watch pipeline itself produces these
  files, so watching them only ever reacts to a DIFFERENT process's writes.
- themes.js: assertCompositionUnchanged(), run before every watch-triggered
  generateEnvironment/styles rerun, re-reads lib/components/internal/environment.json
  fresh and aborts loudly (never silently rewrites, never silently skips) if
  another process has since changed what's actually built -- this remains a
  safety net for legitimate source-file triggers even with the glob fix.

Also, per review of experiment 2 in the same bug report: composition
resolution is now ATOMIC. Previously PRIMARY_THEME and SECONDARY_THEMES each
resolved independently through explicit > persisted > preset, so setting
only one env var could silently mix in a stale persisted value for the
other, producing compositions nobody asked for (including, in the reported
case, the same theme as both primary and secondary). Now: if EITHER env var
is set explicitly, BOTH resolve from explicit-or-preset and persisted is
ignored entirely for both. Stickiness (inheriting the whole last build)
applies only when neither var is set. Setting only one var now logs an
explicit note that the other defaulted to the preset, not to the last build.

Also renames the build's env-var inputs with an AWSUI_ prefix, matching the
existing AWSUI_TARGET convention used internally:
  PRIMARY_THEME -> AWSUI_PRIMARY_THEME
  SECONDARY_THEMES -> AWSUI_SECONDARY_THEMES
  THEME_PRESET -> AWSUI_THEME_PRESET
The emitted build-time constants (lib/components/internal/environment.*'s
PRIMARY_THEME/INCLUDED_THEMES) and the pre-existing ALWAYS_VISUAL_REFRESH env
var keep their unprefixed names unchanged -- only the two new build INPUT
env vars are prefixed, to avoid churning the already-verified persisted
read/write round-trip and avoid a breaking change to ALWAYS_VISUAL_REFRESH.

Verified: reproduced the original clobber on unpatched code with a real
concurrent gulp watch + quick-build, then confirmed the fix eliminates it
(both via the glob exclusion alone, and via the staleness guard for a
legitimate source-file trigger). Added a permanent end-to-end regression
test (themes-persistence.test.js) that starts a stale watch-style process
with one composition, rebuilds with another, and asserts the staleness
message fires and nothing gets clobbered. All four shipped compositions
rebuilt with the new env var names; no-arg quick-build output re-verified
byte-identical to the Phase A baseline.
The old switcher was two independent checkboxes (Visual refresh, One theme)
baked on the assumption "classic is primary, everything else is an opt-in
class" -- which PRIMARY_THEME broke. With AWSUI_PRIMARY_THEME=visual-refresh
the VR checkbox was forced-checked-readonly (meaningless) while classic was
still offered as an option despite never being built at all.

Replaces it with a single <select> over exactly [resolved primary theme,
...compiled secondary themes] -- one option per theme actually in this
build, never more. Selecting the primary applies no theme class (it's
already the default); selecting a secondary applies exactly that theme's
class and removes any other secondary's. This generalizes the old
ALWAYS_VISUAL_REFRESH forced-checked-readonly special case (now: disable
the select when there's only one option) and the old "one-theme cancels
visual-refresh" precedence (now: any two secondaries are mutually exclusive
by construction, since only one can be the active selection).

app-context.tsx: `Theme` (a 2-value enum: Default | OneTheme) becomes
`SELECTABLE_THEMES: string[]`, derived from the build's own PRIMARY_THEME/
INCLUDED_THEMES constants. `theme` is now a plain string id instead of an
enum member. `applyThemeClass`/`isVisualRefreshActive` are generic over
however many secondary themes this build actually compiled.

Runtime substrate is unchanged: window[awsui-visual-refresh-flag],
window[awsui-global-flags].oneTheme, and the .awsui-visual-refresh/
.awsui-one-theme body classes are still set exactly as before -- only how
the active theme id is *computed* changed, not what gets set once it's
known. `urlParams.visualRefresh` is preserved as a derived boolean (two
other pages read it directly: app-layout/with-table-and-sticky-offset and
theming/integration) rather than removed.

Backwards compatible: `?visualRefresh=true` is still accepted as an alias
for `theme=visual-refresh` (only when visual-refresh was actually built);
`theme=` wins when both are present. An unrecognized `theme=<id>` silently
falls back to the primary, matching the previous fallback-to-default
behavior for unrecognized values.

Verified: pages/ is built by buildPages/webpack, not quick-build, so
lib/components and lib/design-tokens are unaffected by this change --
confirmed via the same baseline diff used for the build-parametrization
work (only the 2 legitimate upstream style changes picked up by the most
recent rebase, plus the already-accounted-for PRIMARY_THEME field, differ).
Built and inspected the compiled bundle for all four shipped compositions
(release default, visual-refresh-only, visual-refresh+one-theme, core-only)
-- PRIMARY_THEME/INCLUDED_THEMES bake in correctly and SELECTABLE_THEMES/
THEME_LABELS resolve to exactly the expected option sets in each, including
the disabled-single-option case for the two primary-only compositions.
webpack (dev mode, full type-checking via ts-loader) compiles clean for
every composition. src/__integ__/themes.test.ts's 8 visualRefresh= URLs
were traced by hand against the new parseQuery logic (no browser/webdriver
available in this environment to execute them directly) and resolve to the
same active theme id and body-class state as before in every case.
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