Feat: Rules - rules.raw() API supports #231#232
Conversation
🦋 Changeset detectedLatest commit: c268326 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 |
WalkthroughAdds a non-breaking Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant Rules as rules (export)
participant Impl as rulesImpl
participant Raw as rules.raw
Note over Rules,Impl: Existing callable export preserved
Dev->>Rules: rules(options)
Rules->>Impl: invoke implementation
Impl-->>Dev: result
Note over Raw: New passthrough API (returns options)
Dev->>Raw: rules.raw(options)
Raw-->>Dev: options
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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 #232 by @black7375. Checking if we can fast forward Target branch ( commit 216f6e113a5e357d6fd8a0abe7f02f058d2a62d5 (HEAD -> main, origin/main)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Mon Jul 21 00:00:00 2025 +0900
Fix: CSS - More strict required check `css.with()` #226Pull request ( commit f117959708886a7cdd9a14aec091051fda57419d (pull_request/rules-raw)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Tue Jul 22 00:00:00 2025 +0900
Feat: Rules - `rules.raw()` API supports #231It is possible to fast forward |
|
Triggered from #232 by @black7375. Checking if we can fast forward Target branch ( commit 216f6e113a5e357d6fd8a0abe7f02f058d2a62d5 (HEAD -> main, origin/main)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Mon Jul 21 00:00:00 2025 +0900
Fix: CSS - More strict required check `css.with()` #226Pull request ( commit cfe50dae04a387d0c8263b149aad41f435184d22 (pull_request/rules-raw)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Fri Aug 22 01:15:59 2025 +0900
Feat: Rules - `rules.raw()` API supports #231It 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/rules/index.ts (3)
188-194: Simplify generics for rulesRaw to a single identity genericrulesRaw’s three generic parameters are unnecessary for preserving exact inference. Using a single generic that extends PatternOptions keeps literals intact while reducing complexity and compile time surface.
Apply this diff:
-function rulesRaw< - Variants extends VariantGroups | undefined = undefined, - ToggleVariants extends VariantDefinitions | undefined = undefined, - Props extends ComplexPropDefinitions<PropTarget> | undefined = undefined ->(options: PatternOptions<Variants, ToggleVariants, Props>) { - return options; -} +/** Identity helper to preserve literal inference of PatternOptions. */ +function rulesRaw<T extends PatternOptions<any, any, any>>(options: T): T { + return options; +}
899-1026: Great coverage for rules.raw; add a couple of integration and type assertion casesStrong start validating pass-through behavior. Consider adding:
- defaultVariants + toggles via raw
- mixing variants and toggles via raw
- a direct type assertion that rules.raw is typed as typeof rulesRaw
Apply this diff (appends tests within the existing describe.concurrent("rules.raw()") block):
@@ describe.concurrent("rules.raw()", () => { it("handles simple Rule properties", () => { @@ rules(ruleObj4); // Ensure it can be used with rules() }); + + it("preserves defaultVariants with toggles via raw", () => { + const obj = rules.raw({ + defaultVariants: ["disabled"], + toggles: { + disabled: { textDecoration: "line-through" } + } + }); + const result = rules(obj, debugId); + expect(result()).toMatch(className(debugId, `${debugId}_disabled_true`)); + assert.hasAllKeys(result.classNames.variants, ["disabled"]); + }); + + it("combines variants and toggles via raw", () => { + const obj = rules.raw({ + variants: { + size: { + small: { padding: 12 } + } + }, + toggles: { + rounded: { borderRadius: 999 } + } + }); + const result = rules(obj, debugId); + assert.hasAllKeys(result.classNames.variants, ["size", "rounded"]); + expect(result({ size: "small", rounded: true })).toMatch( + className( + debugId, + `${debugId}_size_small`, + `${debugId}_rounded_true` + ) + ); + }); + + it("exposes correct type for rules.raw", () => { + // Compile-time only: ensures the property is typed as the function itself + assertType<typeof rulesRaw>(rules.raw); + }); });
196-199: Add explicit type annotation to therulesexportThe
rulesexport should be hardened with an explicit intersection type to cement the public API and prevent accidental widening in future refactors. SincerulesImplisn’t referenced or imported anywhere else and the compat layer still exportsrulesasrecipe, this change is safe and non-breaking.You can apply the minimal update:
-export const rules = Object.assign(rulesImpl, { - raw: rulesRaw -}); +export const rules: typeof rulesImpl & { raw: typeof rulesRaw } = Object.assign( + rulesImpl, + { raw: rulesRaw } +);Optional: if you’d like to preserve
rulesImplwithout mutation, wrap it before assignment:- export const rules = Object.assign(rulesImpl, { - raw: rulesRaw - }); +const rulesWrapped = ((...args) => + (rulesImpl as unknown as (...a: typeof args) => ReturnType<typeof rulesImpl>)(...args) +) as typeof rulesImpl; +export const rules: typeof rulesImpl & { raw: typeof rulesRaw } = Object.assign( + rulesWrapped, + { raw: rulesRaw } +);— There are no other imports/usages of
rulesImploutside this file, andpackages/css/src/compat.ts:42still exportsrules as recipe.
📜 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 (1)
packages/css/src/rules/index.ts(4 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 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/rules/index.ts
📚 Learning: 2024-12-03T11:16:32.528Z
Learnt from: black7375
PR: mincho-js/mincho#134
File: packages/transform-to-vanilla/src/transform.ts:49-51
Timestamp: 2024-12-03T11:16:32.528Z
Learning: In projects that target both CommonJS and ES modules, adjusting the TypeScript `module` option may not be feasible. Therefore, using `ts-ignore` comments to suppress TypeScript errors related to `import.meta` is acceptable.
Applied to files:
packages/css/src/rules/index.ts
🧬 Code graph analysis (1)
packages/css/src/rules/index.ts (3)
packages/css/src/compat.ts (6)
VariantGroups(32-32)VariantDefinitions(37-37)ComplexPropDefinitions(35-35)PropTarget(36-36)PatternOptions(33-33)rules(42-42)packages/css/src/index.ts (6)
VariantGroups(35-35)VariantDefinitions(40-40)ComplexPropDefinitions(38-38)PropTarget(39-39)PatternOptions(36-36)rules(29-29)packages/css/src/rules/types.ts (5)
VariantGroups(49-49)VariantDefinitions(40-40)ComplexPropDefinitions(74-76)PropTarget(72-72)PatternOptions(176-191)
🔇 Additional comments (2)
packages/css/src/rules/index.ts (2)
50-61: LGTM: API split into rulesImpl and augmented rules preserves backward compatibilityRenaming the implementation to rulesImpl and keeping the public export as rules (callable) looks good. The generic signature and returned RuntimeFn remain intact. Nice, non-breaking extension path for adding rules.raw.
239-239: Confirmed:assertTypeis supported in Vitest v3.2.2
- A scan of all
package.jsonfiles shows Vitest is only declared once, at the repo root, as^3.2.2.
(script output:/home/jailuser/git/package.json → vitest ^3.2.2)- The Vitest v3.x API includes
assertTypeas part of the testing assertions exposed viaimport.meta.vitest(vitest.dev).No further changes to your test code are required. You may wish to ensure your TypeScript configuration includes the
"vitest/importMeta"type definitions so thatimport.meta.vitest.assertTypeis properly recognized by the compiler.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
.changeset/plenty-turkeys-retire.md (1)
5-7: Fix markdownlint MD036: use a heading instead of bold text.Switch the emphasized "New" to a proper heading to satisfy markdownlint and align with the repo’s style in other changesets.
-**New** +### Newpackages/css/src/rules/index.ts (3)
196-198: Pin the callable+property intersection type forrulesto ensure stable inference across TS versions.While
Object.assigninfers an intersection, being explicit avoids drift and IDE hiccups when navigating types.-export const rules = Object.assign(rulesImpl, { - raw: rulesRaw -}); +export const rules: typeof rulesImpl & { raw: typeof rulesRaw } = + Object.assign(rulesImpl, { raw: rulesRaw });
899-1027: Great coverage forrules.raw(); consider one more case to exercise function-basedcompoundVariantsviaraw.You already cover base, variants, toggles, and props. Adding a function-form
compoundVariantscase viarules.raw()would mirror therules()tests and verify generic inference holds throughrawas well.Here’s a minimal add-on you can drop into this block:
+ it("supports function-based compoundVariants via raw()", () => { + const variants = { + color: { + brand: { color: "#FFFFA0" }, + accent: { color: "#FFE4B5" } + }, + size: { + small: { padding: 12 }, + medium: { padding: 16 }, + large: { padding: 24 } + } + } as const; + + const rawObj = rules.raw({ + variants, + compoundVariants: ({ color, size }) => [ + { condition: [color.brand, size.small], style: { fontSize: 16 } } + ] + }); + // Type-level check still flows through raw() + assertType< + PatternOptions< + { + color: { brand: { color: string }; accent: { color: string } }; + size: { small: { padding: number }; medium: { padding: number }; large: { padding: number } }; + }, + undefined, + undefined + > + >(rawObj); + + const r = rules(rawObj, "raw_compound"); + expect(r({ color: "brand", size: "small" })).toMatch( + className( + "raw_compound", + "raw_compound_color_brand", + "raw_compound_size_small", + "raw_compound_compound_0" + ) + ); + });
50-50: KeeprulesImplInternal
To maintain a minimal public API surface and avoid encouraging consumers to depend on this internal helper, let’s remove theexportkeyword fromrulesImpl.• File needing update:
packages/css/src/rules/index.ts(around line 50)
• Change:-export function rulesImpl< +function rulesImpl<This ensures only the intended
rulessymbol is exported publicly.
📜 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 (2)
.changeset/plenty-turkeys-retire.md(1 hunks)packages/css/src/rules/index.ts(4 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 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/rules/index.ts
📚 Learning: 2024-12-03T11:16:32.528Z
Learnt from: black7375
PR: mincho-js/mincho#134
File: packages/transform-to-vanilla/src/transform.ts:49-51
Timestamp: 2024-12-03T11:16:32.528Z
Learning: In projects that target both CommonJS and ES modules, adjusting the TypeScript `module` option may not be feasible. Therefore, using `ts-ignore` comments to suppress TypeScript errors related to `import.meta` is acceptable.
Applied to files:
packages/css/src/rules/index.ts
🧬 Code graph analysis (2)
.changeset/plenty-turkeys-retire.md (1)
packages/css/src/css/index.ts (2)
cssRaw(109-111)cssWithRaw(122-124)
packages/css/src/rules/index.ts (1)
packages/css/src/rules/types.ts (5)
VariantGroups(49-49)VariantDefinitions(40-40)ComplexPropDefinitions(74-76)PropTarget(72-72)PatternOptions(176-191)
🪛 markdownlint-cli2 (0.17.2)
.changeset/plenty-turkeys-retire.md
5-5: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (2)
packages/css/src/rules/index.ts (2)
188-194: LGTM:rulesRawgeneric passthrough preserves precise types.The generic defaults and passthrough return enable strong inference and align with the existing
cssRawpattern. No runtime impact.
239-239: LGTM:assertTypeimport ensures compile-time coverage for newrules.rawtests.This is a clean way to validate types without affecting runtime.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.changeset/plenty-turkeys-retire.md (2)
5-5: Fix markdownlint MD036: use a heading instead of bold textThe section label is styled with emphasis; switch to a heading to satisfy markdownlint and keep consistency with other notes.
-**New** +## New
7-7: Enrich the release note to clarify scope and stabilityOptional, but adding a short qualifier helps consumers scanning the changelog understand what “raw” does and that it’s additive/non‑breaking. Also consider referencing the related issue.
-- Add `rules.raw()` API +- Add `rules.raw()` API (passthrough returning raw PatternOptions; non‑breaking). +- See #231 for background.
📜 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 (2)
.changeset/plenty-turkeys-retire.md(1 hunks)packages/css/src/rules/index.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/css/src/rules/index.ts
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
.changeset/plenty-turkeys-retire.md
5-5: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (2)
.changeset/plenty-turkeys-retire.md (2)
1-3: Changesets frontmatter looks correct for a minor bumpPackage name and release type are well-formed for Changesets’ frontmatter.
2-2: Confirm “minor” aligns with your release policy for new public APIIf this package is ≥1.0.0, “minor” for additive public API is correct per semver. If still <1.0.0 and you treat “minor” as potentially breaking, double‑check this matches your policy.
|
/fast-forward |
|
Triggered from #232 (comment) by @black7375. Trying to fast forward Target branch ( commit 216f6e113a5e357d6fd8a0abe7f02f058d2a62d5 (HEAD -> main, origin/main)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Mon Jul 21 00:00:00 2025 +0900
Fix: CSS - More strict required check `css.with()` #226Pull request ( commit c268326ae498f7c2d1f0504d517fd4d340a1169f (pull_request/rules-raw)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Tue Jul 22 00:00:00 2025 +0900
Feat: Rules - `rules.raw()` API supports #231Fast forwarding $ git push origin c268326ae498f7c2d1f0504d517fd4d340a1169f:main
To https://github.com/mincho-js/mincho.git
216f6e1..c268326 c268326ae498f7c2d1f0504d517fd4d340a1169f -> main |
|
Triggered from #232 by @black7375. Checking if we can fast forward Target branch ( commit c268326ae498f7c2d1f0504d517fd4d340a1169f (HEAD -> main, origin/main, pull_request/rules-raw)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Tue Jul 22 00:00:00 2025 +0900
Feat: Rules - `rules.raw()` API supports #231Pull request ( commit c268326ae498f7c2d1f0504d517fd4d340a1169f (HEAD -> main, origin/main, pull_request/rules-raw)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Tue Jul 22 00:00:00 2025 +0900
Feat: Rules - `rules.raw()` API supports #231It is possible to fast forward |
1 participant
Description
Implement
rules.raw()APIRelated Issue
Summary by CodeRabbit
New Features
Tests
Chores
Additional context
Checklist