Fix: styled typesafe for tagname, variants #247#252
Fix: styled typesafe for tagname, variants #247#252github-actions[bot] merged 3 commits intomainfrom
styled typesafe for tagname, variants #247#252Conversation
🦋 Changeset detectedLatest commit: bc7dd88 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 #252 by @black7375. Checking if we can fast forward Target branch ( commit d4cde3f82fdb590ee7ec7740c8f2d6e886696cc1 (HEAD -> main, origin/main)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Tue Aug 12 00:00:00 2025 +0900
Docs: Make the changeset format consistent.Pull request ( commit bc7dd88d744fe292db499108074d2a06c49a2063 (pull_request/styled-typesafe)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Sat Aug 16 00:00:00 2025 +0900
Refactor: `styled` code order & namingIt is possible to fast forward |
WalkthroughAdds changeset entries for patch releases. Exports new type PropDefinitionOutput from @mincho-js/css (compat.ts and index.ts). Overhauls @mincho-js/react styled typings: introduces ComponentWithAs, intrinsic inference helpers, and revised StyledComponent generics using PropDefinitionOutput and ResolveComplex. Updates react package.json to include @mincho-js/csstype. Changes
Sequence Diagram(s)Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–75 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
![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 (4)
.changeset/clear-chicken-shout.md (1)
5-6: Use a heading instead of emphasis (MD036).Replace bold-without-heading with a proper heading for lint compliance.
-**type** +### type.changeset/violet-paws-jam.md (1)
5-6: Prefer heading syntax for consistency (avoid emphasis-as-heading).Use a heading for the “export” section.
-**export** +### exportpackages/react/src/index.ts (2)
144-154: Avoid overriding precise ref types; drop RefAttributes.Adding RefAttributes to StyledComponent props eclipses accurate ref inference from ComponentProps/ComponentProps, weakening type-safety.
Preferred: rely on ComponentProps-derived ref, or compute ref from ElementRef.
export type StyledComponent< TProps = Record<string, unknown>, Component extends ElementType = ElementType, Variants extends VariantGroups | undefined = undefined, ToggleVariants extends VariantDefinitions | undefined = undefined, Props extends ComplexPropDefinitions<PropTarget> | undefined = undefined > = ComponentWithAs< - TProps & - RefAttributes<unknown> & - PatternOptionsToProps<Variants, ToggleVariants, Props>, + TProps & PatternOptionsToProps<Variants, ToggleVariants, Props>, Component >;Alternatively (if you want an explicit ref in Props), use ElementRef:
- import ElementRef from react
- use RefAttributes<ElementRef> instead of unknown.
156-166: Prop resolution shape looks sane; minor nit on unknown widening.PatternOptionsToProps falls back to unknown via NonNever, which can widen resulting prop types in edge cases. If you prefer stricter fallback, consider Record<never, never> instead of unknown to avoid surfacing “any-like” props.
-type NonNever<T> = [T] extends [never] ? unknown : T; +type NonNever<T> = [T] extends [never] ? Record<never, never> : T;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (6)
.changeset/clear-chicken-shout.md(1 hunks).changeset/violet-paws-jam.md(1 hunks)packages/css/src/compat.ts(1 hunks)packages/css/src/index.ts(1 hunks)packages/react/package.json(1 hunks)packages/react/src/index.ts(1 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/index.tspackages/css/src/compat.ts
🧬 Code graph analysis (1)
packages/react/src/index.ts (2)
packages/css/src/index.ts (9)
RulesVariants(33-33)VariantGroups(35-35)ComplexPropDefinitions(38-38)PropTarget(39-39)PatternOptions(36-36)ResolveComplex(43-43)VariantObjectSelection(42-42)ConditionalVariants(41-41)PropDefinitionOutput(44-44)packages/react/src/runtime.ts (1)
$$styled(17-81)
🪛 markdownlint-cli2 (0.18.1)
.changeset/clear-chicken-shout.md
5-5: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (4)
packages/react/package.json (1)
66-68: Dependency placement looks right; please verify public type usage.If your generated .d.ts references types from @mincho-js/csstype, keeping it in dependencies is correct. If it’s only used internally and not exposed, it can be a devDependency instead.
packages/css/src/index.ts (1)
43-45: LGTM: new public type export is consistent with usage in React typings.Exporting PropDefinitionOutput here aligns with the styled typing changes downstream.
packages/css/src/compat.ts (1)
41-43: LGTM: compat surface mirrors main export.PropDefinitionOutput addition here keeps the compat API in sync with index.ts. Recipe export remains intact. Based on learnings.
packages/react/src/index.ts (1)
104-116: Runtime throw in styled: ensure build-time transform is guaranteed.styled unconditionally throws at runtime. If a transform (Babel/Vite/Rollup/TS) rewrites styled calls to $$styled, you’re fine; otherwise consumers will crash.
- Verify the repo includes and documents the required transform.
- Consider providing a runtime-safe alias (e.g., exporting $$styled as styled while retaining overload typings) if feasible in your architecture.
Would you like guidance on a type-only overload + runtime alias pattern to avoid hard runtime throws in dev builds?
|
/fast-forward |
|
Triggered from #252 (comment) by @black7375. Trying to fast forward Target branch ( commit d4cde3f82fdb590ee7ec7740c8f2d6e886696cc1 (HEAD -> main, origin/main)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Tue Aug 12 00:00:00 2025 +0900
Docs: Make the changeset format consistent.Pull request ( commit bc7dd88d744fe292db499108074d2a06c49a2063 (pull_request/styled-typesafe)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Sat Aug 16 00:00:00 2025 +0900
Refactor: `styled` code order & namingFast forwarding $ git push origin bc7dd88d744fe292db499108074d2a06c49a2063:main
To https://github.com/mincho-js/mincho.git
d4cde3f..bc7dd88 bc7dd88d744fe292db499108074d2a06c49a2063 -> main |
1 participant
Description
Type safe implement
styled.Related Issue
styledmakes to type-safe #247Summary by CodeRabbit
New Features
Chores
Refactor
Additional context
Checklist