Feat: defineRules - properties and shorcuts#308
Conversation
🦋 Changeset detectedLatest commit: d601036 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 #308 by @black7375. Checking if we can fast forward Target branch ( commit d245af329da086591d564171e10f939e053a84f7 (HEAD -> main, origin/main)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Tue Dec 2 00:00:00 2025 +0900
chore: update depsPull request ( commit 0fcc79b1193fe498f7c637262f7b0e84a4a7f879 (pull_request/define-rules)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Sun Feb 15 00:00:00 2026 +0900
Feat: `defineRules` - funtional properties and shortcutsIt is possible to fast forward |
📝 WalkthroughWalkthroughAdds a new defineRules utility (implementation + extensive TypeScript types) to the CSS package, updates package exports to expose it, and includes two changeset metadata files; also adds a spell-check word in VSCode settings. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/big-hands-rescue.md:
- Line 5: Update the changeset summary line to correct the typo: replace
"shorcuts" with "shortcuts" so the note reads "`defineRules`: Properties and
shortcuts"; edit the .changeset/big-hands-rescue.md file and update the text
accordingly.
In @.changeset/six-coins-ask.md:
- Line 5: Update the changeset note to fix the spelling mistake: replace the
word "shorcuts" with "shortcuts" in the sentence that lists `defineRules` so the
release note reads "`defineRules` - functional properties and shortcuts".
In `@packages/css/src/defineRules/index.ts`:
- Around line 139-142: The code currently assigns disabled properties (propDef
=== false) because only isPlainObject(propDef) is checked; update the branch to
skip assignment when propDef is strictly false by adding an explicit check
before assigning to out: if (propDef === false) return; then keep the existing
isPlainObject check and assignment for non-object defs. Adjust the block around
isPlainObject(propDef), propDef, prop, value, and out so that disabled defs are
not applied at runtime (or optionally throw instead of returning if you prefer
failing fast).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.changeset/big-hands-rescue.md.changeset/six-coins-ask.md.vscode/settings.jsonpackages/css/src/defineRules/index.tspackages/css/src/defineRules/types.tspackages/css/src/index.ts
.changeset/six-coins-ask.md
Outdated
| "@mincho-js/css": minor | ||
| --- | ||
|
|
||
| - `defineRules` - functional properties and shorcuts |
There was a problem hiding this comment.
Fix the typo in the changeset note.
“shorcuts” → “shortcuts” for release-note clarity.
✏️ Suggested fix
- `defineRules` - functional properties and shorcuts
+ `defineRules` - functional properties and shortcuts📝 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.
| - `defineRules` - functional properties and shorcuts | |
| - `defineRules` - functional properties and shortcuts |
🧰 Tools
🪛 LanguageTool
[grammar] ~5-~5: Ensure spelling is correct
Context: ...efineRules` - functional properties and shorcuts
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.changeset/six-coins-ask.md at line 5, Update the changeset note to fix the
spelling mistake: replace the word "shorcuts" with "shortcuts" in the sentence
that lists `defineRules` so the release note reads "`defineRules` - functional
properties and shortcuts".
| // just assign => last one wins | ||
| if (isPlainObject(propDef) === false) { | ||
| (out as Record<string, unknown>)[prop] = value; | ||
| return; |
There was a problem hiding this comment.
Disabled properties (false) are still applied at runtime.
This conflicts with the type contract where false means “no values allowed.” Consider skipping assignment (or throwing) when the property is disabled.
🔧 Suggested fix
- // just assign => last one wins
- if (isPlainObject(propDef) === false) {
+ if (propDef === false) {
+ return;
+ }
+ // just assign => last one wins
+ if (isPlainObject(propDef) === false) {
(out as Record<string, unknown>)[prop] = value;
return;
}📝 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.
| // just assign => last one wins | |
| if (isPlainObject(propDef) === false) { | |
| (out as Record<string, unknown>)[prop] = value; | |
| return; | |
| if (propDef === false) { | |
| return; | |
| } | |
| // just assign => last one wins | |
| if (isPlainObject(propDef) === false) { | |
| (out as Record<string, unknown>)[prop] = value; | |
| return; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/css/src/defineRules/index.ts` around lines 139 - 142, The code
currently assigns disabled properties (propDef === false) because only
isPlainObject(propDef) is checked; update the branch to skip assignment when
propDef is strictly false by adding an explicit check before assigning to out:
if (propDef === false) return; then keep the existing isPlainObject check and
assignment for non-object defs. Adjust the block around isPlainObject(propDef),
propDef, prop, value, and out so that disabled defs are not applied at runtime
(or optionally throw instead of returning if you prefer failing fast).
black7375
force-pushed
the
define-rules
branch
from
0fcc79b to
d601036
Compare
|
Triggered from #308 by @black7375. Checking if we can fast forward Target branch ( commit d245af329da086591d564171e10f939e053a84f7 (HEAD -> main, origin/main)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Tue Dec 2 00:00:00 2025 +0900
chore: update depsPull request ( commit d60103698a0cfe301d13a972de1d2ffdcc024fd2 (pull_request/define-rules)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Sun Feb 15 00:00:00 2026 +0900
Feat: `defineRules` - funtional properties and shortcutsIt is possible to fast forward |
|
/fast-forward |
|
Triggered from #308 (comment) by @black7375. Trying to fast forward Target branch ( commit d245af329da086591d564171e10f939e053a84f7 (HEAD -> main, origin/main)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Tue Dec 2 00:00:00 2025 +0900
chore: update depsPull request ( commit d60103698a0cfe301d13a972de1d2ffdcc024fd2 (pull_request/define-rules)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Sun Feb 15 00:00:00 2026 +0900
Feat: `defineRules` - funtional properties and shortcutsFast forwarding $ git push origin d60103698a0cfe301d13a972de1d2ffdcc024fd2:main
To https://github.com/mincho-js/mincho.git
d245af3..d601036 d60103698a0cfe301d13a972de1d2ffdcc024fd2 -> main |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/react-babel/tsconfig.app.json`:
- Line 17: The tsconfig types array includes "vitest/importMeta" but the
examples/react-babel project doesn't list vitest as a dependency; either add
vitest to devDependencies in package.json (so the "vitest/importMeta" type
reference resolves) or remove "vitest/importMeta" from the "types" array in
tsconfig.app.json; locate the "types" entry and update it accordingly to keep
only installed type packages (e.g., keep "vite/client" or add "vitest" to
devDependencies).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.changeset/big-hands-rescue.md.changeset/six-coins-ask.md.vscode/settings.jsonexamples/react-babel/tsconfig.app.jsonpackages/css/src/defineRules/index.tspackages/css/src/defineRules/types.ts
✅ Files skipped from review due to trivial changes (1)
- .changeset/six-coins-ask.md
🚧 Files skipped from review as they are similar to previous changes (4)
- .changeset/big-hands-rescue.md
- packages/css/src/defineRules/index.ts
- .vscode/settings.json
- packages/css/src/defineRules/types.ts
| "moduleDetection": "force", | ||
| "noEmit": true, | ||
| "jsx": "react-jsx", | ||
| "types": ["vitest/importMeta", "vite/client"], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for vitest/vite dependencies across repo package.json files
fd package.json -x rg -n '"vitest"|"vite"' {}Repository: mincho-js/mincho
Length of output: 112
🏁 Script executed:
cat -n examples/react-babel/tsconfig.app.jsonRepository: mincho-js/mincho
Length of output: 1296
🏁 Script executed:
cat -n examples/react-babel/package.json 2>/dev/null || echo "No package.json in examples/react-babel"Repository: mincho-js/mincho
Length of output: 2072
🏁 Script executed:
head -50 package.jsonRepository: mincho-js/mincho
Length of output: 2750
Add vitest to devDependencies or remove vitest/importMeta from types array.
The vite/client type is available (vite is in devDependencies), but vitest/importMeta references a package not listed in examples/react-babel's dependencies. Declare vitest as a devDependency or remove the reference from the types array.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/react-babel/tsconfig.app.json` at line 17, The tsconfig types array
includes "vitest/importMeta" but the examples/react-babel project doesn't list
vitest as a dependency; either add vitest to devDependencies in package.json (so
the "vitest/importMeta" type reference resolves) or remove "vitest/importMeta"
from the "types" array in tsconfig.app.json; locate the "types" entry and update
it accordingly to keep only installed type packages (e.g., keep "vite/client" or
add "vitest" to devDependencies).
1 participant
Description
property & shorcut support
Related Issue
Summary by CodeRabbit
New Features
Chores
Additional context
Checklist