Feat: theme - Support raw method#271
Conversation
🦋 Changeset detectedLatest commit: 50145b3 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 |
WalkthroughA minor version bump introduces a new Changes
Sequence DiagramsequenceDiagram
participant Caller as Semantic Token Getter
participant Resolver as Token Resolution
participant RawExtractor as Raw Extractor
participant VarStore as Collected Vars
Caller->>Resolver: semantic getter invoked
Caller->>RawExtractor: this.raw(varReference)
RawExtractor->>RawExtractor: detect if var(...) reference
alt var(...) reference found
RawExtractor->>VarStore: lookup concrete value
VarStore-->>RawExtractor: return concrete value
else not a var reference
RawExtractor->>RawExtractor: return input as-is
end
RawExtractor-->>Caller: concrete value or input
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 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 |
|
Triggered from #271 by @black7375. Checking if we can fast forward Target branch ( commit 743384fe107bc2ce523f3b1879690e8e81dfdc24 (HEAD -> main, origin/main)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Wed Sep 24 00:00:00 2025 +0900
Feat: `theme` - Support `fallbackVar` method #266Pull request ( commit 50145b3eca703e9884113b58eecd0c6ecb8631d3 (pull_request/theme-raw-api)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Thu Sep 25 00:00:00 2025 +0900
Feat: `theme` - Support `raw` method #268It is possible to fast forward |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.changeset/dull-bugs-send.md (1)
6-6: Use a proper heading instead of bold emphasis.The bold text
**theme**should be formatted as a proper Markdown heading for better document structure.Apply this diff:
-**theme** +## theme
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/dull-bugs-send.md(1 hunks)packages/css/src/theme/index.ts(9 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
.changeset/dull-bugs-send.md (1)
packages/css/src/theme/types.ts (2)
myTheme(331-415)it(223-416)
packages/css/src/theme/index.ts (2)
packages/css/src/utils.ts (1)
getVarName(20-29)packages/transform-to-vanilla/src/types/style-rule.ts (1)
CSSVarValue(119-119)
🪛 markdownlint-cli2 (0.18.1)
.changeset/dull-bugs-send.md
6-6: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (4)
packages/css/src/theme/index.ts (4)
136-142: LGTM!The raw utility is correctly attached to resolvedTokens with appropriate property descriptors, following the same pattern as fallbackVar.
487-504: LGTM! Verify return type annotation.The raw extractor logic correctly handles var() references and edge cases. Ensure the return type is updated to match the corrected interface return type (
CSSVarValueinstead of implicitunknown).
704-729: Test expectations correctly updated.The test expectations are consistently updated to reflect the new primitive value handling, where numbers, booleans, and undefined are no longer stringified. These changes provide good coverage of the behavioral change.
Also applies to: 753-777, 1228-1263
897-947: Excellent test coverage for raw value extraction.This test comprehensively validates the new
raw()API, covering var() reference extraction, non-var value pass-through, and usage with different token types. The test cases effectively demonstrate the intended functionality.
|
|
||
| interface ThemeSubFunctions { | ||
| fallbackVar: typeof fallbackVar; | ||
| raw(varReference: unknown): string; |
There was a problem hiding this comment.
Fix return type to match actual implementation.
The return type is declared as string, but the implementation returns CSSVarValue, which can be string | number | boolean | undefined (as seen in line 610 where primitives are returned directly). This type mismatch undermines type safety.
Apply this diff to fix the type signature:
- raw(varReference: unknown): string;
+ raw(varReference: unknown): CSSVarValue;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| raw(varReference: unknown): string; | |
| raw(varReference: unknown): CSSVarValue; |
🤖 Prompt for AI Agents
In packages/css/src/theme/index.ts around line 38, the raw(varReference:
unknown): string; signature is incorrect because the implementation returns
CSSVarValue (string | number | boolean | undefined); change the declared return
type to CSSVarValue to match the implementation and restore type safety (i.e.,
raw(varReference: unknown): CSSVarValue;).
| function extractCSSValue(value: TokenValue): CSSVarValue { | ||
| if (isPrimitive(value)) { | ||
| return String(value) as CSSVarValue; | ||
| return value as CSSVarValue; | ||
| } |
There was a problem hiding this comment.
Verify type consistency with CSSVarValue definition.
Primitives are now returned directly instead of being stringified. However, CSSVarValue is defined as `${string | number}` (a template literal type that resolves to string), which is inconsistent with returning primitive number, boolean, or undefined values.
Consider one of these approaches:
- Update the CSSVarValue type definition in
packages/transform-to-vanilla/src/types/style-rule.ts:
export type CSSVarValue = string | number | boolean | undefined;- Or revert to stringifying primitives if CSS variable values must be strings:
- return value as CSSVarValue;
+ return String(value) as CSSVarValue;The first approach aligns with the test expectations and new behavior, while the second maintains type consistency with the current CSSVarValue definition.
🤖 Prompt for AI Agents
In packages/css/src/theme/index.ts around lines 608 to 611, extractCSSValue
currently returns primitive values (number/boolean/undefined) but CSSVarValue is
defined as a template literal (`${string | number}`) which resolves to string
and is therefore inconsistent; fix by updating the CSSVarValue type in
packages/transform-to-vanilla/src/types/style-rule.ts to be a union that allows
primitives (e.g., string | number | boolean | undefined), update any
imports/usages if necessary, run typecheck/tests, and remove any code that
previously stringified primitives if present.
|
/fast-forward |
|
Triggered from #271 (comment) by @black7375. Trying to fast forward Target branch ( commit 743384fe107bc2ce523f3b1879690e8e81dfdc24 (HEAD -> main, origin/main)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Wed Sep 24 00:00:00 2025 +0900
Feat: `theme` - Support `fallbackVar` method #266Pull request ( commit 50145b3eca703e9884113b58eecd0c6ecb8631d3 (pull_request/theme-raw-api)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Thu Sep 25 00:00:00 2025 +0900
Feat: `theme` - Support `raw` method #268Fast forwarding $ git push origin 50145b3eca703e9884113b58eecd0c6ecb8631d3:main
To https://github.com/mincho-js/mincho.git
743384f..50145b3 50145b3eca703e9884113b58eecd0c6ecb8631d3 -> main |
1 participant
Description
Add
theme'sraw()methodRelated Issue
Summary by CodeRabbit
Release Notes
theme()function andraw()method for accessing concrete values from semantic tokensAdditional context
Checklist