Repository navigation
feat: per-theme color-scheme hints and theme-name validation (v3.10.0) - #5
Merged
Merged
Conversation
- Add NgColorScheme, NgThemeOption, NgThemeInput, ResolvedTheme types - normalizeThemeInputs() converts string/object theme arrays to ResolvedTheme[] with per-theme scheme hints - provideThemeStack() accepts string or object form (mixable), produces resolvedThemes on NgConfig - CoreThemeService.applyColorSchemeHint() uses per-theme lookup: auto/none removes the hint, light/dark sets it - Public services expose themeNames: string[] for template binding - ng-add schematic prompts for scheme hints, renders object-form entries, passes schemeMap to the anti-flash script - Anti-flash script sets color-scheme hint before Angular boots when schemeMap is provided - Sync schematic detects and preserves object-form themes - Demo app: added sepia theme with scheme: 'light' - Specs: types, config, core service, services, anti-flash Verified: tsc --noEmit clean for lib, schematics, and spec configs.
Adds the public API reference for the object-form theme entries
({ name, scheme? }) and the NgColorScheme union, plus a table of
scheme values and their effect on the root element's color-scheme.
Two independent problems made `pnpm test` unusable: 1. jsdom@28.1.0 requires undici v7's subpath layout (lib/handler/wrap-handler.js), which undici v8 removed. The workspace hoisted undici@8.11.2 (via release-it), so the vitest worker crashed at startup with MODULE_NOT_FOUND and no test ever ran. Scoped the override to jsdom so release-it keeps undici 8.x. 2. The library declared @angular/core and @angular/common as peerDependencies, so pnpm auto-installed a second copy (@angular/core@22.0.4) under projects/ngx-theme-stack/node_modules while the root had 22.2.1. The Angular unit-test runner initializes TestBed from the root copy, so every spec using TestBed failed with 'Need to call TestBed.initTestEnvironment() first'. Set autoInstallPeers: false so the library resolves Angular from the workspace root (single module instance). Verified: pnpm test goes from a worker crash (0 tests run) to 110 passing; pnpm run build:lib and pnpm run build:demo succeed.
provideThemeStack() returned the raw user `themes` array instead of the merged, deduplicated list (built-ins + custom), so NgConfig.themes lost the built-in themes. The internal resolvedThemes array already carried the correct data, but the public field regressed. Also: - CoreThemeService falls back to deriving resolvedThemes from themes when a config is injected without it, so a hand-rolled NGX_THEME_STACK_CONFIG provider cannot silently empty the valid-theme set. - Use T[] instead of Array<T> in the two places the new types introduced (eslint @typescript-eslint/array-type). Verified: pnpm test 110 passing.
The sync schematic extracted `themes: [...]` and split the array body on
commas, which shredded object-form entries
({ name: 'sepia', scheme: 'light' }) into broken fragments and rewrote
app.config.ts to invalid TypeScript on the next build.
Extracted the parsing into schematics/utils/theme-input.ts, which splits on
top-level commas only (quote- and bracket-aware) and parses both 'name' and
{ name, scheme } entries with order-independent keys.
Verified: pnpm run build:demo now succeeds and a second sync is idempotent;
14 new specs cover the parser.
The skill generator still described themes as string-only, so an agent following it would add a custom theme without a scheme hint and leave native widgets (scrollbars, form controls) on the OS colour scheme. Adds the object form and the scheme table to the API reference, documents themeNames/resolvedThemes, and turns the custom-theme question into a decision the agent must ask (light or dark native widgets) before generating config.
The root config matches 'files: ["**/*.ts"]' and ESLint flat config does not
ignore dist or build caches by default, so any glob that reaches them reports
errors on code nobody wrote:
- dist/**.d.ts -> 8 errors (Angular emits #private fields and any)
- .agents/** -> 3 component-selector errors (generated skill example
components use an app- selector, correct for a consumer
app, but the workspace rule demands the lib prefix)
Per-project lint already scopes to projects/** so the normal command passes;
this closes the latent trap for IDEs, editor plugins, and root-level globs.
Verified: eslint run directly on those paths now reports 0 errors, real source
is still linted (an injected component-selector error is still caught),
pnpm run lint and both per-project lints pass, tests 110+1, build:lib OK.
publish.yml chose the dist-tag from a single heuristic: whether the ref name
contained a '-'. Two holes let unstable code become 'latest', which is what
npm install ngx-theme-stack resolves for everyone:
- a stable tag created outside main (e.g. v4.0.0 on a feature branch) has no
'-', so it published to latest;
- workflow_dispatch runs on any branch, and ref_name ('dev') has no '-', so
the manual path published to latest from wherever it was dispatched.
Adds two guards before install/build/publish:
- a stable tag must be an ancestor of origin/main;
- a manual dispatch must run from main.
Prereleases are untouched: a tag containing '-' skips both guards and still
publishes under the 'next' dist-tag.
Also sets fetch-depth: 0 on checkout. The ancestor check needs origin/main, and
the default fetch-depth: 1 leaves no remote-tracking refs, which would have made
the guard fail closed and reject every valid release.
Verified with scripts/verify-publish-guards.mjs, which extracts the guard run
bodies from the workflow itself and executes them against real SHAs: stable tag
on main passes, stable tag off main fails, prerelease skips both guards, manual
dispatch from dev fails, manual dispatch from main passes.
…ate none
'none' was a second spelling for 'auto': both call
removeProperty('color-scheme'). The scheme was a prerelease-only value that
never reached a stable release, so it is now removed:
NgColorScheme = 'light' | 'dark' | 'auto'
The scheme prompt validated nothing and buildProvideCall interpolated the answer
verbatim, so typing 'banana' produced an app that does not compile:
error TS2322: Type '"banana"' is not assignable to type 'NgColorScheme | undefined'
Now the prompt lists light/dark/auto with what each does, states that anything
else falls back to 'auto', and warns when it does. Membership is decided by a new
parseColorScheme() in schematics/ng-add/constants.ts, duplicated from the library
on purpose because schematics are CommonJS and cannot import the ESM library.
Sync applies the same rule: an invalid scheme in an existing config is dropped so
the entry behaves as scheme-less, and each dropped value is reported through a
warning instead of being silently rewritten.
Verified: 117 tests (was 110), tsc clean for the three configs, lint clean,
build:lib and build:demo OK, sync regenerated the demo anti-flash script.
…chematic copies
Nothing validated a theme name: provideThemeStack only checked for empty, while
types.ts documented one pattern and the anti-flash script enforced another. A bad
name degraded silently:
2lucky .2lucky is invalid CSS, so the browser drops the rule and the
theme loses its styles
lucky theme classList.add throws InvalidCharacterError
lucky.theme .lucky.theme matches two classes while the class token is the
single string 'lucky.theme', so the styles never apply, silently
The rule becomes /^-?[a-zA-Z_][a-zA-Z0-9_-]*$/ and is now enforced. It also
accepts _lucky and -lucky, which are valid CSS but the old pattern rejected,
causing an unnecessary first-paint flash. Non-ASCII names stay rejected on
purpose: a documented narrower subset, not an accident.
provideThemeStack() throws NgxThemeStackError, the ng-add prompt re-asks with the
reason, and sync reports invalid names it finds in an existing config.
The anti-flash inline script keeps its own check. It was proposed to drop it as
redundant with v.indexOf(t) !== -1, but the value comes from localStorage and the
list it is compared against is a hand-editable array in index.html; it runs
before Angular and writes to classList and to an attribute. It now uses the same
pattern, so it no longer flashes _lucky / -lucky.
The values the schematics share with the library were duplicated behind KEEP IN
SYNC comments, which is a promise rather than a mechanism. A single source file
cannot belong to both compile units, because the schematics use
rootDir: "schematics" with CommonJS while ng-packagr builds ESM from src/:
error TS6059: File '.../src/lib/types.ts' is not under 'rootDir' '.../schematics'
scripts/generate-schematic-constants.mjs now generates
schematics/ng-add/generated-config.ts from the library, wired to prebuild. It
reads the TypeScript AST rather than matching text, exposes the pattern as a
string that both sides compile with new RegExp(...), and replaces four duplicated
value sets (DEFAULT_THEMES, DEFAULTS, COLOR_SCHEMES, the name pattern).
src/schematics/generated-config.spec.ts fails when the copy goes stale; that was
verified by tampering with the generated file and watching the suite fail.
Verified: 138 tests (was 117), tsc clean for the three configs, lint clean,
build:lib and build:demo OK, publish guards 5/5, generator idempotent, and the
demo's re-synced inline script uses the new pattern.
…mpt strict
Two loose ends from the theme-name work.
1. An unknown scheme could still reach the DOM. normalizeThemeInputs passed
entry.scheme through untouched, so a hand-written config — which reaches the
runtime without the compiler having checked it — could put an arbitrary string
into style.setProperty('color-scheme', ...).
Fixed on both sides:
- normalizeThemeInputs replaces a scheme outside NG_COLOR_SCHEMES with 'auto',
so resolvedThemes is authoritative;
- applyColorSchemeHint now uses a positive whitelist. Only 'light' and 'dark'
are written; everything else removes the hint, which is what 'auto' means.
The check is no longer "scheme === auto", so a value that bypassed
normalization clears the hint instead of being written.
2. The agent-skill prompt read "Generate an AI Agent Skill? [Y/n]" with
"input !== 'n'", so any typo (asdf) counted as yes. New parseYesNo() accepts
y/yes/n/no case-insensitively, takes the prompt default on an empty answer, and
returns undefined for anything else so askYesNo() can re-ask instead of
guessing. The helper is a pure function, so it is unit tested.
Verified: 147 tests (was 138), tsc clean for the three configs, lint clean,
build:lib and build:demo OK, publish guards 5/5.
…ll version The generated skill was mechanically in sync with its template (regenerating produced no diff), but the template itself had fallen behind the code: - the API reference did not mention the theme-name rule, while provideThemeStack has thrown NgxThemeStackError for a bad name since 5c9044e. A consumer reading the skill would be surprised by a build error the docs never warned about; - the sync entry did not say that it now reports an invalid name or scheme instead of silently rewriting it; - metadata.version was still 1.2.2 after adding the object-form themes, the scheme table and the themeNames/resolvedThemes fields, which made the field meaningless. Bumped to 1.3.0. Verified: tsc clean, 147 tests, lint clean, build:lib and build:demo OK, publish guards 5/5, regenerated output has no literal escapes and still contains no 'none' scheme.
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.
Closes #4
Summary
Merge
devintomainfor the v3.10.0 release line (currentlyv3.10.0-next.1on dev). Headline features:color-schemehints — object-form themes ({ name, scheme }), normalizedresolvedThemes,color-schemehint applied on<html>.THEME_NAME_PATTERNvalidated inprovideThemeStackand schematics; build error instead of silent first-paint flash.scripts/generate-schematic-constants.mjsbridges the ESM/CJS split, with a spec asserting sync.Changes
src/lib/types.tsNgThemeOption,ResolvedTheme,normalizeThemeInputs,implicitScheme,THEME_NAME_SOURCE/PATTERN,NG_COLOR_SCHEMESsrc/lib/config/index.tsprovideThemeStackvalidates names, merges built-ins + user themes, exposesresolvedThemessrc/lib/core/core-theme.service.tslight/darkonly), strict anti-flash guardssrc/lib/services/*themeNamespassthrough for select/cycleschematics/ng-add/*schematics/utils/*theme-input.tsparser (object-form safe), sharedbuildAntiFlashScriptwith scheme hintsschematics/sync, skillscripts/*generate-schematic-constants.mjs,verify-publish-guards.mjsci/publishpublish.ymlguards;fix(ci)keeps unstable builds offlatestdemothemes.csstokens,app.config.ts,index.htmlregenerateddocsTest plan
npx ng lint ngx-theme-stack— all files passnpx ng test ngx-theme-stack --watch=false— 147/147 pass (11 files)npx ng build ngx-theme-stack— builds clean