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

Fix TypeScript error with function interpolations #3175

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

Conversation

Cerber-Ursi
Copy link
Contributor

What:

After recent change to CSSTypes (#3141, #3164), some uses of styled components, namely the ones which provided a function interpolation, started to fail to typecheck. This adds a couple of tests for these use-cases and a corresponding fix.

Why:

This fixes #3174.

How:

As hinted on by @Andarist, the reason for the bug is that TypeScript caches the inference data from the failed overloads, even if this forces otherwise-matching ones to fail too. Therefore, the fix is to add "dummy" overload, never taken due to having a never-typed field in it, to guide type inference correctly.

Checklist:

  • Documentation (N/A)
  • Tests
  • Code complete
  • Changeset

Copy link

changeset-bot bot commented Apr 6, 2024

🦋 Changeset detected

Latest commit: 696acf8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@emotion/styled Patch

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

Copy link

codesandbox-ci bot commented Apr 6, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Comment on lines +66 to +67
// 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.
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;
   }
 `

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Typescript issue with CSSObject in version 11.11.5 of @emotion/styled
2 participants