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.

Sign up for GitHub

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

Jump to bottom

Fix TypeScript error with function interpolations #3175

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix TypeScript error with function interpolations #3175

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
182f56d
add tests for different ways to pass styles; add never-used `styled` …
Cerber-Ursi Apr 6, 2024
696acf8
yarn changeset
Cerber-Ursi Apr 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/thin-bears-talk.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@emotion/styled': patch
---

Fix incorrectly inferred types when using function interpolations
10 changes: 10 additions & 0 deletions packages/styled/types/base.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,16 @@ export interface CreateStyledComponent<
SpecificComponentProps extends {} = {},
JSXProps extends {} = {}
> {
// This signature is actually never taken (due to `marker: never`); it's here to guide type inference.
// See https://github.com/emotion-js/emotion/issues/3174 for context.
Comment on lines +66 to +67
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a fan of this - mainly because it introduces more work to the compiler. Overloads are matched from the top to the bottom so this definitely results in some typechecking perf hit. I understand that this might be inevitable though.

Can we think of any alternative solution that would reorder overloads back but would prevent the first ones from matching tagged template calls?

I've done some testing and it seems that the main problem that we have here is that interpolated components become inference sources for AdditionalProps or something like that. When I cast them to strings then all tests pass - see the diff below

git diff 
diff --git a/packages/styled/types/base.d.ts b/packages/styled/types/base.d.ts
index b8e1c93e..05efbaea 100644
--- a/packages/styled/types/base.d.ts
+++ b/packages/styled/types/base.d.ts
@@ -63,28 +63,10 @@ export interface CreateStyledComponent<
   SpecificComponentProps extends {} = {},
   JSXProps extends {} = {}
 > {
-  // This signature is actually never taken (due to `marker: never`); it's here to guide type inference.
-  // See https://github.com/emotion-js/emotion/issues/3174 for context.
-  (
-    ...styles: Array<
-      Interpolation<
-        ComponentProps & SpecificComponentProps & { theme: Theme }
-      > & { marker: never }
-    >
-  ): StyledComponent<ComponentProps, SpecificComponentProps, JSXProps>
-
-  (
-    template: TemplateStringsArray,
-    ...styles: Array<
-      Interpolation<ComponentProps & SpecificComponentProps & { theme: Theme }>
-    >
-  ): StyledComponent<ComponentProps, SpecificComponentProps, JSXProps>
-
   /**
    * @typeparam AdditionalProps  Additional props to add to your styled component
    */
-  <AdditionalProps extends {}>(
-    template: TemplateStringsArray,
+  <AdditionalProps extends {} = {}>(
     ...styles: Array<
       Interpolation<
         ComponentProps &
@@ -98,10 +80,18 @@ export interface CreateStyledComponent<
     JSXProps
   >
 
+  (
+    template: TemplateStringsArray,
+    ...styles: Array<
+      Interpolation<ComponentProps & SpecificComponentProps & { theme: Theme }>
+    >
+  ): StyledComponent<ComponentProps, SpecificComponentProps, JSXProps>
+
   /**
    * @typeparam AdditionalProps  Additional props to add to your styled component
    */
-  <AdditionalProps extends {} = {}>(
+  <AdditionalProps extends {}>(
+    template: TemplateStringsArray,
     ...styles: Array<
       Interpolation<
         ComponentProps &
diff --git a/packages/styled/types/tests.tsx b/packages/styled/types/tests.tsx
index 14b7378a..09eb0d5c 100644
--- a/packages/styled/types/tests.tsx
+++ b/packages/styled/types/tests.tsx
@@ -90,16 +90,16 @@ const StyledOriginal = styled(Original, {
 const Label = styled.label``
 const Button = styled.button``
 const Input = styled.input`
-  & + ${Label}: {
+  & + ${String(Label)}: {
     margin-left: 3px;
   }
 `
 
 const Input2 = styled.input`
-  & + ${Label}: {
+  & + ${String(Label)}: {
     margin-left: 3px;
   }
-  & + ${Button}: {
+  & + ${String(Button)}: {
     margin-left: 3px;
   }
 `

(
...styles: Array<
Interpolation<
ComponentProps & SpecificComponentProps & { theme: Theme }
> & { marker: never }
>
): StyledComponent<ComponentProps, SpecificComponentProps, JSXProps>

(
template: TemplateStringsArray,
...styles: Array<
Expand Down
29 changes: 29 additions & 0 deletions packages/styled/types/tests.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,3 +301,32 @@ const Input5 = styled.input`
// $ExpectError
styled('div', { shouldForwardProp: (prop: 'foo') => true })({})
}

{
// Different ways to provide props to styled components

styled.div`
textAlign: ${'center'},
position: ${'relative'},
`

styled.div({
textAlign: `center`,
position: `relative`
})

styled.div(() => ({
textAlign: `center`,
position: `relative`
}))

styled('div')({
textAlign: `center`,
position: `relative`
})

styled('div')(() => ({
textAlign: `center`,
position: `relative`
}))
}