Feat: theme - css variable reference #263#265
Conversation
🦋 Changeset detectedLatest commit: 6482954 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Triggered from #265 by @black7375. Checking if we can fast forward Target branch ( commit 7488b27101a6a646dfe0136966e76df4672a74d3 (HEAD -> main, origin/main)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Sat Sep 20 00:00:00 2025 +0900
Chore: use tsgo in vscodePull request ( commit 6482954eb32b6a69037d45ba1f4f0da5b80a411c (pull_request/theme-reference-api)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Tue Sep 23 00:00:00 2025 +0900
Feat: `theme` - Support `this.` type #263It is possible to fast forward |
WalkthroughThis PR introduces a two-pass token resolution system for theming in Changes
Sequence Diagram(s)sequenceDiagram
participant User as User Code
participant GlobalTheme as globalTheme()
participant Pass1 as Pass 1: assignTokenVariables
participant Pass2 as Pass 2: resolveSemanticTokens
participant CSS as CSS Output
User->>GlobalTheme: Input tokens + selector
GlobalTheme->>Pass1: Process all tokens
rect rgb(220, 240, 255)
Note over Pass1: Assign CSS variables<br/>for each token<br/>(create placeholders for refs)
end
Pass1->>Pass2: Token graph complete
rect rgb(240, 220, 255)
Note over Pass2: Resolve semantic getters<br/>by evaluating against graph
end
Pass2->>CSS: Return CSS vars + resolved tokens
CSS-->>User: [className, ResolveThemeOutput]
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
black7375
changed the title
theme - css variable refernce #263theme - css variable reference #263
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/css/src/theme/index.ts (1)
479-500: Consider logging a warning when using the color fallback.The fallback to
#000000(line 499) for invalid color objects could silently mask configuration errors. Consider adding a console warning when this fallback is used so developers are alerted to malformed color tokens.// Fallback for invalid color object + console.warn(`Invalid color object, falling back to #000000:`, value); return "#000000" as CSSVarValue;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.changeset/petite-bats-lick.md(1 hunks)configs/tsconfig-custom/tsconfig.base.json(1 hunks)packages/css/src/css/index.ts(11 hunks)packages/css/src/rules/index.ts(29 hunks)packages/css/src/theme/index.ts(25 hunks)packages/css/src/theme/types.ts(8 hunks)packages/css/src/utils.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
packages/css/src/theme/index.ts (4)
packages/css/src/theme/types.ts (3)
Theme(8-10)ResolveTheme(154-160)TokenValue(140-140)packages/css/src/types.ts (1)
Resolve(1-3)packages/transform-to-vanilla/src/types/style-rule.ts (3)
CSSVarValue(119-119)PureCSSVarKey(117-117)PureCSSVarFunction(122-124)packages/css/src/utils.ts (3)
camelToKebab(31-33)getVarName(20-29)identifierName(5-11)
packages/css/src/rules/index.ts (2)
packages/css/src/utils.ts (1)
identifierName(5-11)packages/css/src/rules/createRuntimeFn.ts (1)
variants(98-102)
packages/css/src/theme/types.ts (2)
packages/transform-to-vanilla/src/index.ts (2)
NonNullableString(8-8)NonNullableString(10-10)packages/transform-to-vanilla/src/types/style-rule.ts (1)
PureCSSVarFunction(122-124)
packages/css/src/css/index.ts (1)
packages/css/src/utils.ts (1)
identifierName(5-11)
🪛 markdownlint-cli2 (0.18.1)
.changeset/petite-bats-lick.md
5-5: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (16)
.changeset/petite-bats-lick.md (1)
1-6: LGTM!The changeset correctly documents the minor version bump and describes the new theme reference feature. The markdown linter warning about line 5 using emphasis instead of a heading is a false positive—this is the standard changeset format for this repository.
packages/css/src/css/index.ts (1)
16-16: LGTM! Clean API rename fromclassNametoidentifierName.The import statement correctly updates to the renamed function, and all usages throughout the test suite have been consistently updated.
packages/css/src/utils.ts (1)
5-11: LGTM! Function renamed for better clarity.The rename from
classNametoidentifierNameis more semantically accurate since this function generates regex patterns for matching identifiers, not just class names.packages/css/src/rules/index.ts (1)
13-13: LGTM! Import statement correctly updated.The import and all usages throughout the file have been consistently updated to use
identifierNameinstead ofclassName.packages/css/src/theme/index.ts (7)
32-70: LGTM! Well-designed public API for the new two-pass token resolution system.The updated signatures use
constgenerics for better type inference andThisTypeto enable semantic token references via getters. This is a breaking change to the public API, but it's appropriately documented in the changeset.The use of
ThisType<ResolveThemeOutput<ThemeTokens>>is particularly elegant—it allows theme tokens with getters to reference other resolved tokens viathis, enabling semantic token patterns like:{ color: { base: { blue: "#0000ff" }, semantic: { get primary() { return this.color.base.blue; } } } }
95-131: LGTM! Clear two-pass orchestration.The algorithm cleanly separates variable assignment (Pass 1) from semantic token resolution (Pass 2). The shared
TokenProcessingContextwithcssVarMapensures consistent CSS variable names across both passes.
148-267: LGTM! Comprehensive Pass 1 implementation handles all token types.The implementation correctly:
- Uses
Object.getOwnPropertyDescriptorsto detect getters vs. regular properties- Creates placeholder references for getters (resolved in Pass 2)
- Handles structured token values (dimension, duration, etc.)
- Recursively processes nested themes
- Preserves getter semantics on TokenCompositeValue's
resolvedpropertyThe
TokenCompositeValuehandling (lines 221-248) is particularly well done—it extracts the composed value and individual properties while preserving the getter for theresolvedproperty in the output.
274-336: LGTM! Pass 2 correctly resolves semantic token references.The key insight is line 294:
descriptor.get.call(resolvedTokens)invokes the getter with the resolved token structure asthis, allowing semantic tokens to reference other tokens' CSS variables. The computed var() reference is then stored in bothvars(for CSS output) andresolvedTokens(for type-safe API).
338-413: LGTM! Comprehensive type guards correctly identify token shapes.The
isStructuredTokenValuefunction (lines 366-386) is particularly important—it determines whether a token's$valueshould be processed as a single unit vs. recursed into as a nested object. This correctly handles:
dimension/durationwith{value, unit}structurecubicBezierarraysfontFamilystrings/arrayscolorobjects with color space definitions
415-459: LGTM! Path utilities ensure consistent variable names and safe nested access.The caching in
getCSSVarByPath(lines 420-429) is crucial—it ensures that the same path always maps to the same CSS variable name across both passes, even thoughcreateVar()generates unique identifiers with hashes.
591-1210: LGTM! Excellent test coverage with proper hash normalization.The test utilities (lines 610-656) elegantly solve the problem of testing hashed CSS variable names:
stripHashremoves hash suffixes for comparisonnormalizeVarsandnormalizeResolvedTokensrecursively normalize the outputvalidateHashFormatensures variables have the expected structureThe comprehensive test suite validates:
- All primitive and structured token types
- Nested themes and arrays
- Semantic token references via getters
- TokenCompositeValue with resolved getters
- Complex mixed structures
packages/css/src/theme/types.ts (4)
1-46: LGTM! Well-structured token definition type hierarchy.The new type structure provides:
TokenDefinitionBase<TokenType, TokenValue>- Generic base for all specialized token definitionsTokenAllDefinition- Union type covering all specialized token types- Getter variant in
ThemeValue- Enables semantic token referencesThis design provides excellent type safety and enables the specialized token handling in the theme resolution logic.
48-138: LGTM! Specialized token definitions follow W3C Design Tokens spec.Each definition properly extends
TokenDefinitionBasewith:
- Literal
$typefor discriminated union behavior- Correctly typed
$valuefor each token type- Optional
$descriptioninherited from baseThe definitions cover:
color- String or complex color space objectdimension- Numeric value with unitfontFamily- String or array of font familiesfontWeight- Numeric or named weightduration- Time value with unitcubicBezier- Four-number array for easingnumber- Plain numeric value
277-329: LGTM! Comprehensive test coverage for specialized token definitions.The test validates that each specialized token definition:
- Satisfies its specific interface (e.g.,
TokenColorDefinition)- Extends the union type
TokenAllDefinitionThis ensures the type hierarchy is correctly structured and all definitions are properly included in the union.
154-172: Readonly removal is intentional and necessary for token construction.Line 155 removes
readonlyfromResolveThemebecause theresolvedTokensobject must be mutable during the two-pass token resolution process:
assignTokenVariablesusessetByPath(resolvedTokens, currentPath, varRef)to populate the structureresolveSemanticTokenscalls getters withdescriptor.get.call(resolvedTokens)for token referencesThe object is only mutated during construction in
assignTokensWithPrefix(lines 127-128) and returned as immutable in practice. No mutations were found in the codebase after the object is returned, andObject.freezeis not applied. While the theoretical risk of consumer mutations exists, the current implementation relies on TypeScript typing and usage patterns to maintain immutability semantics.configs/tsconfig-custom/tsconfig.base.json (1)
103-103: LGTM! EnablingnoImplicitThisis verified safe.Enabling this TypeScript compiler option improves type safety by preventing implicit
anytype onthis. The theme changes in this PR use explicitThisType<...>annotations, which aligns well with this stricter checking. Compilation verification confirms no implicit this errors are introduced.
|
/fast-forward |
|
Triggered from #265 (comment) by @black7375. Trying to fast forward Target branch ( commit 7488b27101a6a646dfe0136966e76df4672a74d3 (HEAD -> main, origin/main)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Sat Sep 20 00:00:00 2025 +0900
Chore: use tsgo in vscodePull request ( commit 6482954eb32b6a69037d45ba1f4f0da5b80a411c (pull_request/theme-reference-api)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Tue Sep 23 00:00:00 2025 +0900
Feat: `theme` - Support `this.` type #263Fast forwarding $ git push origin 6482954eb32b6a69037d45ba1f4f0da5b80a411c:main
To https://github.com/mincho-js/mincho.git
7488b27..6482954 6482954eb32b6a69037d45ba1f4f0da5b80a411c -> main |
1 participant
Description
theme supports with reference
Related Issue
Summary by CodeRabbit
New Features
theme()and support for reference variables in theming system.Improvements
Additional context
Checklist