Feat: rules.with() API implement #234 #242
Conversation
🦋 Changeset detectedLatest commit: aafcd1c 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 new Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Consumer
participant CSS as css (callable + helpers)
participant Rules as rules (callable + helpers)
participant Core as Internal helpers
Note over CSS,Rules: New `with` helper attached to callable exports
Dev->>CSS: css.with(config)(baseRule)
CSS->>Core: cssWith → cssWithRaw / cssWithMultiple
Core-->>Dev: composed RuntimeFn / rule
Dev->>Rules: rules.with(transform)(options)
Rules->>Core: rulesWith → rulesWithRaw / rulesWithMultiple
Core-->>Dev: rule map / RuntimeFn
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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. Comment |
|
Triggered from #242 by @black7375. Checking if we can fast forward Target branch ( commit 6f7b9801c216a3ae4d7a2359a78bbad6427aa63b (HEAD -> main, origin/main)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Mon Jul 28 00:00:00 2025 +0900
Feat: rulesVariant Funtion #154
Co-authored-by: Jeong-jj <rgfdds98@gmail.com>Pull request ( commit f360c85c47b2c5ca9bb85e607a80453cc78fd478 (pull_request/css-rule-with)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Tue Jul 29 00:00:00 2025 +0900
Feat: `rules.with()` API implement #234It 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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/eager-lemons-wink.md(1 hunks)packages/css/src/css/index.ts(2 hunks)packages/css/src/rules/index.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 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.tspackages/css/src/css/index.ts
🧬 Code graph analysis (3)
.changeset/eager-lemons-wink.md (1)
packages/transform-to-vanilla/src/types/style-rule.ts (2)
CSSPropertiesWithConditions(45-47)CSSPropertiesWithVars(49-52)
packages/css/src/rules/index.ts (7)
packages/css/src/compat.ts (13)
rules(42-42)VariantGroups(32-32)VariantDefinitions(37-37)ComplexPropDefinitions(35-35)PropTarget(36-36)PatternOptions(33-33)CSSRule(47-47)CSSRuleWith(26-26)RuntimeFn(31-31)ConditionalVariants(38-38)createVar(10-10)fallbackVar(11-11)css(23-23)packages/css/src/rules/types.ts (9)
VariantGroups(52-52)VariantDefinitions(43-43)ComplexPropDefinitions(77-79)PropTarget(75-75)PatternOptions(179-194)RuntimeFn(201-208)ConditionalVariants(168-177)PropDefinition(80-85)RecipeStyleRule(31-31)packages/transform-to-vanilla/src/types/style-rule.ts (3)
CSSRule(33-35)PureCSSVarKey(129-129)CSSPropertiesWithConditions(45-47)packages/css/src/css/types.ts (1)
CSSRuleWith(9-23)packages/transform-to-vanilla/src/utils/object.ts (1)
mergeObject(5-11)packages/css/src/utils.ts (2)
getDebugName(13-15)getVarName(20-29)packages/css/src/css/index.ts (1)
css(105-109)
packages/css/src/css/index.ts (3)
packages/css/src/compat.ts (3)
css(23-23)cssMultiple(24-24)ComplexCSSRule(46-46)packages/transform-to-vanilla/src/types/style-rule.ts (1)
ComplexCSSRule(26-26)packages/transform-to-vanilla/src/transform.ts (1)
transform(16-42)
🪛 markdownlint-cli2 (0.18.1)
.changeset/eager-lemons-wink.md
5-5: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (3)
packages/css/src/css/index.ts (3)
115-118: LGTM: cssImpl remains the single execution path.Calls
transformthenvStylewithdebugId, consistent with existing usage and tests.
137-137: Good generic widening on css.with().multiple().Using
Data[keyof Data]andkeyof Dataaligns overloads withcssMultipleand improves inference for mixed key/value maps.
105-109: Drop the cssImpl wrapper; mutation is safe
cssImplisn’t publicly exported, soObject.assign(cssImpl, …)can’t leak to consumers. Retain the warning about destructuringwith—usecss.with(or rename on import) instead.Likely an incorrect or invalid review comment.
| **css** | ||
|
|
||
| ## New | ||
| - Add `rules.with()` API No newline at end of file |
There was a problem hiding this comment.
Fix markdownlint MD036: use a heading instead of emphasis.
Switch the emphasized "css" line to a proper heading to satisfy markdownlint and keep changesets consistent.
-**css**
+### css📝 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.
| **css** | |
| ## New | |
| - Add `rules.with()` API | |
| ### css | |
| ## New | |
| - Add `rules.with()` API |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
5-5: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🤖 Prompt for AI Agents
In .changeset/eager-lemons-wink.md around lines 5 to 8, replace the emphasized
line "**css**" with a proper Markdown heading (for example "### css") so it uses
a heading level instead of emphasis; update that single line and keep the rest
of the changeset content unchanged.
black7375
force-pushed
the
css-rule-with
branch
from
f360c85 to
aafcd1c
Compare
|
Triggered from #242 by @black7375. Checking if we can fast forward Target branch ( commit 6f7b9801c216a3ae4d7a2359a78bbad6427aa63b (HEAD -> main, origin/main)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Mon Jul 28 00:00:00 2025 +0900
Feat: rulesVariant Funtion #154
Co-authored-by: Jeong-jj <rgfdds98@gmail.com>Pull request ( commit aafcd1c4380d6778f7ee1864c08a56f46944ae91 (pull_request/css-rule-with)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Tue Jul 29 00:00:00 2025 +0900
Feat: `rules.with()` API implement #234It is possible to fast forward |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/eager-lemons-wink.md(1 hunks)packages/css/src/css/index.ts(2 hunks)packages/css/src/rules/index.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/css/src/css/index.ts
🧰 Additional context used
🧠 Learnings (1)
📚 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
🧬 Code graph analysis (2)
packages/css/src/rules/index.ts (7)
packages/css/src/index.ts (13)
rules(30-30)VariantGroups(35-35)VariantDefinitions(40-40)ComplexPropDefinitions(38-38)PropTarget(39-39)PatternOptions(36-36)CSSRule(5-5)CSSRuleWith(29-29)RuntimeFn(34-34)ConditionalVariants(41-41)createVar(17-17)fallbackVar(18-18)css(28-28)packages/css/src/rules/types.ts (9)
VariantGroups(52-52)VariantDefinitions(43-43)ComplexPropDefinitions(77-79)PropTarget(75-75)PatternOptions(179-194)RuntimeFn(201-208)ConditionalVariants(168-177)PropDefinition(80-85)RecipeStyleRule(31-31)packages/transform-to-vanilla/src/types/style-rule.ts (2)
CSSRule(33-35)PureCSSVarKey(129-129)packages/css/src/css/types.ts (1)
CSSRuleWith(9-23)packages/transform-to-vanilla/src/utils/object.ts (1)
mergeObject(5-11)packages/css/src/utils.ts (2)
getDebugName(13-15)getVarName(20-29)packages/css/src/css/index.ts (1)
css(105-109)
.changeset/eager-lemons-wink.md (1)
packages/transform-to-vanilla/src/types/style-rule.ts (4)
CSSPropertiesWithConditions(45-47)CSSPropertiesWithVars(49-52)CSSRule(33-35)CSSComplexProperties(54-56)
🪛 markdownlint-cli2 (0.18.1)
.changeset/eager-lemons-wink.md
5-5: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (1)
.changeset/eager-lemons-wink.md (1)
5-5: Switch the emphasized section label to a headingmarkdownlint (MD036) still fires on the
**css**line—please turn it into a heading (e.g.### css) so the changeset passes lint.
| return rulesMultiple( | ||
| data, | ||
| (value: Parameters<typeof callback>[0], key: keyof Data) => | ||
| mapData(callback(value) as Parameters<MapData>[0], key), | ||
| debugId | ||
| ) as TransformDataMapping<Data, MapData>; | ||
| } else { | ||
| const ruleMap = ruleMapOrData as Data; | ||
| const debugId = mapDataOrDebugId; | ||
| return rulesMultiple( | ||
| ruleMap, | ||
| callback as unknown as MapData, | ||
| debugId | ||
| ) as TransformRuleMap<Data>; |
There was a problem hiding this comment.
Fix the rules.with().multiple mapData plumbing
In the mapData overload you’re calling mapData(callback(value) …), but callback returns a PatternOptions object while mapData is typed (and, at runtime, written) to receive a RestrictedCSSRule. Because of the cast, TS stays quiet but the runtime value no longer matches the declared contract—any consumer that expects to read CSS properties (per the signature) will instead see a PatternOptions shape and produce nonsense output (e.g. trying to spread the rule into { base: rule } ends up nesting base.base). In practice this makes rules.with(cb).multiple(data, mapData) unusable as soon as the second argument looks at the rule it’s been promised.
Please feed mapData the actual RulePattern type and update the overload/return type accordingly so both the types and runtime value line up. For example:
- function rulesWithMultiple<
- Data extends Record<string | number, RestrictedCSSRule>,
- MapData extends (
- value: Data[keyof Data],
- key: keyof Data
- ) => PatternOptions<
- VariantGroups | undefined,
- VariantDefinitions | undefined,
- ComplexPropDefinitions<PropTarget> | undefined
- >
- >(
- data: Data,
- mapData: MapData,
- debugId?: string
- ): TransformDataMapping<Data, MapData>;
+ function rulesWithMultiple<
+ Data extends Record<string | number, RestrictedCSSRule>,
+ MapData extends (
+ value: RulePattern,
+ key: keyof Data
+ ) => PatternOptions<
+ VariantGroups | undefined,
+ VariantDefinitions | undefined,
+ ComplexPropDefinitions<PropTarget> | undefined
+ >
+ >(
+ data: Data,
+ mapData: MapData,
+ debugId?: string
+ ): TransformRuleMap<Data>;
@@
- return rulesMultiple(
- data,
- (value: Parameters<typeof callback>[0], key: keyof Data) =>
- mapData(callback(value) as Parameters<MapData>[0], key),
- debugId
- ) as TransformDataMapping<Data, MapData>;
+ return rulesMultiple(
+ data,
+ (value: RestrictedCSSRule, key: keyof Data) =>
+ mapData(callback(value), key),
+ debugId
+ ) as TransformRuleMap<Data>;This keeps the behaviour aligned with the API contract instead of swapping the data out from under the consumer.
📝 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.
| return rulesMultiple( | |
| data, | |
| (value: Parameters<typeof callback>[0], key: keyof Data) => | |
| mapData(callback(value) as Parameters<MapData>[0], key), | |
| debugId | |
| ) as TransformDataMapping<Data, MapData>; | |
| } else { | |
| const ruleMap = ruleMapOrData as Data; | |
| const debugId = mapDataOrDebugId; | |
| return rulesMultiple( | |
| ruleMap, | |
| callback as unknown as MapData, | |
| debugId | |
| ) as TransformRuleMap<Data>; | |
| // --- overload signatures for rulesWithMultiple --- | |
| // before: | |
| - function rulesWithMultiple< | |
| - Data extends Record<string | number, RestrictedCSSRule>, | |
| - MapData extends ( | |
| - value: Data[keyof Data], | |
| - key: keyof Data | |
| - ) => PatternOptions< | |
| - VariantGroups | undefined, | |
| - VariantDefinitions | undefined, | |
| - ComplexPropDefinitions<PropTarget> | undefined | |
| - > | |
| - >( | |
| - data: Data, | |
| - mapData: MapData, | |
| - debugId?: string | |
| - ): TransformDataMapping<Data, MapData>; | |
| // after: | |
| function rulesWithMultiple< | |
| Data extends Record<string | number, RestrictedCSSRule>, | |
| MapData extends ( | |
| value: RulePattern, | |
| key: keyof Data | |
| ) => PatternOptions< | |
| VariantGroups | undefined, | |
| VariantDefinitions | undefined, | |
| ComplexPropDefinitions<PropTarget> | undefined | |
| > | |
| >( | |
| data: Data, | |
| mapData: MapData, | |
| debugId?: string | |
| ): TransformRuleMap<Data>; | |
| // --- implementation of the mapData branch --- | |
| // before: | |
| return rulesMultiple( | |
| data, | |
| (value: Parameters<typeof callback>[0], key: keyof Data) => | |
| mapData(callback(value) as Parameters<MapData>[0], key), | |
| debugId | |
| ) as TransformDataMapping<Data, MapData>; | |
| // after: | |
| return rulesMultiple( | |
| data, | |
| (value: RestrictedCSSRule, key: keyof Data) => | |
| mapData(callback(value), key), | |
| debugId | |
| ) as TransformRuleMap<Data>; |
🤖 Prompt for AI Agents
In packages/css/src/rules/index.ts around lines 119–132, the mapData overload
incorrectly calls mapData(callback(value)...) passing a PatternOptions-shaped
value into a function typed/expected to receive a RestrictedCSSRule/RulePattern;
change the plumbing to pass an actual RulePattern into mapData (i.e., feed
mapData the rule pattern produced by the rule factory rather than the callback's
PatternOptions), and update the overload signatures/return types so the second
argument to rulesMultiple is typed as (rule: RulePattern, key: keyof Data) =>
MapData (and adjust TransformDataMapping/TransformRuleMap types accordingly) so
the runtime shape and TypeScript signatures align.
|
/fast-forward |
|
Triggered from #242 (comment) by @black7375. Trying to fast forward Target branch ( commit 6f7b9801c216a3ae4d7a2359a78bbad6427aa63b (HEAD -> main, origin/main)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Mon Jul 28 00:00:00 2025 +0900
Feat: rulesVariant Funtion #154
Co-authored-by: Jeong-jj <rgfdds98@gmail.com>Pull request ( commit aafcd1c4380d6778f7ee1864c08a56f46944ae91 (pull_request/css-rule-with)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Tue Jul 29 00:00:00 2025 +0900
Feat: `rules.with()` API implement #234Fast forwarding $ git push origin aafcd1c4380d6778f7ee1864c08a56f46944ae91:main
To https://github.com/mincho-js/mincho.git
6f7b980..aafcd1c aafcd1c4380d6778f7ee1864c08a56f46944ae91 -> main |
1 participant
Description
Implement
rules.with()APIRelated Issue
Summary by CodeRabbit
Additional context
Checklist