Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change styled() types so that it works with libraries like Framer Motion #1417

Open
jalooc opened this issue Jul 12, 2024 · 1 comment
Open
Labels
enhancement: proposal 💬 Improvement of current behaviour that needs to be discussed needs: triage 🏷 Issue needs to be checked and prioritized

Comments

@jalooc
Copy link

jalooc commented Jul 12, 2024

Describe the enhancement

When extending custom components, Linaria's types require that component to have the style prop [link] (kudos for setting that requirement in the type system - really helpful and solid 💪).

This works with usual custom components, but fails to work with e.g. Framer Motion. I believe this is a result of 2 things:

  1. Linaria requiring the styles prop to be CSSProperties [link] (which is fair),
  2. Framer Motion's style prop to be an extension of CSSProperties allowing for passing motion values to styles [link].

Granted, the problem lies probably more on the FM's extension of styles, but I figured it'd be easier for Linaria to relax the styles type requirement, considering that (if I understand correctly) the only purpose that Linaria requires styles is for is setting the custom properties. So the type could be relaxed to a mere Record<string, unknown> maybe?

Motivation

Framer Motion is a great animation library with an api allowing for fantastic components composition/extension with the api that Linaria (and similar css-in-js libs) provide. Would be great if Linaria's types worked with it seamlessly, just like e.g. Styled Components' types do.

@jalooc jalooc added the enhancement: proposal 💬 Improvement of current behaviour that needs to be discussed label Jul 12, 2024
@github-actions github-actions bot added the needs: triage 🏷 Issue needs to be checked and prioritized label Jul 12, 2024
@jalooc
Copy link
Author

jalooc commented Jul 12, 2024

So the type could be relaxed to a mere Record<string, unknown> maybe?

It actually seems to be working - I used patch-package to apply this patch to @linaria/[email protected]:

diff --git a/node_modules/@linaria/react/types/styled.d.ts b/node_modules/@linaria/react/types/styled.d.ts
index c790c46..f6fa008 100644
--- a/node_modules/@linaria/react/types/styled.d.ts
+++ b/node_modules/@linaria/react/types/styled.d.ts
@@ -11,9 +11,9 @@ type Has<T, TObj> = [T] extends [TObj] ? T : T & TObj;
 export declare const omit: <T extends Record<string, unknown>, TKeys extends keyof T>(obj: T, keys: TKeys[]) => Omit<T, TKeys>;
 declare function styled(componentWithStyle: () => any): (error: 'The target component should have a className prop') => void;
 declare function styled<TProps extends Has<TMustHave, {
-    style?: React.CSSProperties;
+    style?: Record<string, unknown>;
 }>, TMustHave extends {
-    style?: React.CSSProperties;
+    style?: Record<string, unknown>;
 }, TConstructor extends Component<TProps>>(componentWithStyle: TConstructor & Component<TProps>): ComponentStyledTagWithInterpolation<TProps, TConstructor>;
 declare function styled<TProps extends Has<TMustHave, {
     className?: string;

and the problem is gone, while the style prop on Linaria components remains type-safe. I'll keep monitoring for the issue while writing more code / corner cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement: proposal 💬 Improvement of current behaviour that needs to be discussed needs: triage 🏷 Issue needs to be checked and prioritized
Projects
None yet
Development

No branches or pull requests

1 participant