Conversation
|
WalkthroughIntroduces CSSRuleWith as a replacement for RestrictCSSRule across CSS type utilities, updates cssWith typing to use CSSRuleWith, wraps VariantStyle values when they are CSSRule with CSSRuleWith, adjusts public exports (including aliasing StyleRuleWith and RecipeVariants), and adds vitest-gated type tests. Changes
Sequence Diagram(s)Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Triggered from #237 by @black7375. Checking if we can fast forward Target branch ( commit c268326ae498f7c2d1f0504d517fd4d340a1169f (HEAD -> main, origin/main)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Tue Jul 22 00:00:00 2025 +0900
Feat: Rules - `rules.raw()` API supports #231Pull request ( commit 432b68d9b638c4f662ccaffdfba184a97d39f22f (pull_request/css-rule-with)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Thu Jul 24 00:00:00 2025 +0900
Feat: `CSSRuleWith` type #236It 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: 0
🧹 Nitpick comments (3)
packages/css/src/css/types.ts (1)
10-10: Consider excluding keys whenfalseappears in unions.
T[K] extends false ? never : Konly excludes when the type is exactlyfalse. If you intend “any presence of false means forbidden,” preferfalse extends T[K] ? never : K.Apply:
- [K in keyof T as T[K] extends false ? never : K]: true extends T[K] + [K in keyof T as false extends T[K] ? never : K]: true extends T[K]packages/css/src/index.ts (1)
33-33: AliasRulesVariants→RecipeVariants: confirm BC.This preserves the commonly used
RecipeVariantsname. If any consumers also importRulesVariantsdirectly from here, consider exporting both for a transition period.packages/css/src/css/index.ts (1)
114-117: Type the fallback callback to avoid union widening.Without annotation,
cssFunctioncan widen toRestrictedCSSRule | ComplexCSSRule. Type the fallback for cleaner inference.Apply:
-function cssWith<const T extends CSSRule>( - callback?: (style: CSSRuleWith<T>) => ComplexCSSRule -) { - type RestrictedCSSRule = CSSRuleWith<T>; - const cssFunction = callback ?? ((style: RestrictedCSSRule) => style); +function cssWith<const T extends CSSRule>( + callback?: (style: CSSRuleWith<T>) => ComplexCSSRule +) { + type RestrictedCSSRule = CSSRuleWith<T>; + const cssFunction: (style: RestrictedCSSRule) => ComplexCSSRule = + callback ?? ((style) => style);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
packages/css/src/compat.ts(1 hunks)packages/css/src/css/index.ts(2 hunks)packages/css/src/css/types.ts(2 hunks)packages/css/src/index.ts(1 hunks)packages/css/src/rules/types.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: black7375
PR: mincho-js/mincho#113
File: packages/css/src/index.ts:33-33
Timestamp: 2024-10-09T13:00:38.449Z
Learning: `recipe` is properly exported from './rules' in `packages/css/src/rules/index.ts`.
📚 Learning: 2024-10-09T13:00:38.449Z
Learnt from: black7375
PR: mincho-js/mincho#113
File: packages/css/src/index.ts:33-33
Timestamp: 2024-10-09T13:00:38.449Z
Learning: `recipe` is properly exported from './rules' in `packages/css/src/rules/index.ts`.
Applied to files:
packages/css/src/index.tspackages/css/src/css/index.tspackages/css/src/compat.tspackages/css/src/rules/types.ts
🧬 Code graph analysis (4)
packages/css/src/css/index.ts (3)
packages/css/src/compat.ts (2)
CSSRuleWith(26-26)ComplexCSSRule(46-46)packages/css/src/css/types.ts (1)
CSSRuleWith(9-23)packages/transform-to-vanilla/src/types/style-rule.ts (2)
ComplexCSSRule(26-26)CSSRule(33-35)
packages/css/src/compat.ts (1)
packages/transform-to-vanilla/src/types/style-rule.ts (3)
CSSRule(33-35)CSSPropertiesWithVars(49-52)CSSPropertiesWithConditions(45-47)
packages/css/src/rules/types.ts (2)
packages/transform-to-vanilla/src/types/style-rule.ts (4)
CSSRule(33-35)CSSPropertiesWithVars(49-52)CSSPropertiesWithConditions(45-47)CSSComplexProperties(54-56)packages/css/src/css/types.ts (1)
CSSRuleWith(9-23)
packages/css/src/css/types.ts (3)
packages/css/src/compat.ts (2)
CSSRuleWith(26-26)CSSRule(47-47)packages/css/src/index.ts (2)
CSSRuleWith(29-29)CSSRule(5-5)packages/transform-to-vanilla/src/types/style-rule.ts (1)
CSSRule(33-35)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (8)
packages/css/src/css/types.ts (2)
9-15: Good rename and improved truthiness check.Switching to
true extends T[K]correctly handles unions containingtrueand fixes the prior narrow check. TheNonNullable<CSSRule[K]>coercion is also a good guardrail.
25-33: ESM-only output (package.jsontype: "module"); no CommonJS build is emitted, soimport.meta.vitestblocks are valid at runtime.packages/css/src/compat.ts (2)
26-26: Alias looks right.
CSSRuleWithre-exported asStyleRuleWithmaintains the compat surface.
28-41: No downstream imports ofRecipeVariantsfound; safe to remove from compat exports.packages/css/src/index.ts (1)
29-29: ExposeCSSRuleWithat top-level.Good addition; keeps the main entrypoint aligned with the new type.
packages/css/src/css/index.ts (1)
16-16: Updated import aligns with new type.Switching to
CSSRuleWithis consistent with the rest of the PR.packages/css/src/rules/types.ts (2)
9-9: Correct import location forCSSRuleWith.Pulling from
../css/types.jskeeps the rules layer decoupled from transform internals.
37-40: Wrapping CSS-rule variants withCSSRuleWithis a solid safety net.Only
CSSRule-shaped variant values are constrained; string/classname and complex values remain untouched. This matches intended ergonomics.
|
/fast-forward |
|
Triggered from #237 (comment) by @black7375. Trying to fast forward Target branch ( commit c268326ae498f7c2d1f0504d517fd4d340a1169f (HEAD -> main, origin/main)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Tue Jul 22 00:00:00 2025 +0900
Feat: Rules - `rules.raw()` API supports #231Pull request ( commit 432b68d9b638c4f662ccaffdfba184a97d39f22f (pull_request/css-rule-with)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Thu Jul 24 00:00:00 2025 +0900
Feat: `CSSRuleWith` type #236Fast forwarding $ git push origin 432b68d9b638c4f662ccaffdfba184a97d39f22f:main
To https://github.com/mincho-js/mincho.git
c268326..432b68d 432b68d9b638c4f662ccaffdfba184a97d39f22f -> main |
1 participant
Description
CSSRuleWithtype &VariantStyleupdateRelated Issue
Summary by CodeRabbit
New Features
Refactor
Additional context
Checklist