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

Type errors when interpolating into tagged template literal #188

Open
molily opened this issue Jul 30, 2024 · 2 comments · May be fixed by #354
Open

Type errors when interpolating into tagged template literal #188

molily opened this issue Jul 30, 2024 · 2 comments · May be fixed by #354
Assignees
@brijeshb42
Labels
package: system Specific to @mui/system status: waiting for maintainer These issues haven't been looked at yet by a maintainer
Milestone
Road to v1

Comments

@molily
Copy link

molily commented Jul 30, 2024 •
edited by github-actions bot
Loading

Steps to reproduce

Hello, first of all, thank you for this great project!

Context: We have a larger Next.js app with many Styled Components. We're currently figuring out if and how certain Styled Components syntax features can be represented in Pigment.

I'm currently testing what can be interpolated when using the tagged template syntax styled.tag`…`, specifically interpolating a string, a function and an object.

(I know that Pigment favors the object notation for these cases and I agree this is a more flexible and powerful syntax. I understand that you need to focus your energies on the recommended syntax. Coming from a Styled Components code base though, I'd like to assess to what extent Pigment supports the tagged template syntax. I'm not sticking to any particular syntax in the long term, just want to find out what works with minimal changes and what requires an architectural change.)

Let's say I have these reusable styles:

const borderString = `
  border: 1px solid red;
`;

const borderRadiusObject: CSSProperties = {
  borderRadius: '5px'
};

When I interpolate the object into a component and also pass a function to access a theme token:

const ButtonWithInterpolation = styled.button`
  ${(): CSSProperties => borderRadiusObject}
  ${({ theme }): CSSProperties => ({ color: theme.color })}
  color: red;
`;

This works as expected (all styles are applied) and there is no TS error on the declaration. At the place of usage with children though:

<ButtonWithInterpolation>ButtonWithInterpolation</ButtonWithInterpolation>

TypeScript 5.3.3 (NOT the latest TypeScript 5.5.4) raises an error:

No overload matches this call.
  Overload 1 of 2, '(props: PolymorphicComponentProps<SxProp, Substitute<DetailedHTMLProps<ButtonHTMLAttributes<HTMLButtonElement>, HTMLButtonElement>, (value: string, index: number, array: readonly string[]) => unknown>, undefined, object>): Element', gave the following error.
    Property 'name' is missing in type '{ children: string; }' but required in type 'Omit<Substitute<Substitute<DetailedHTMLProps<ButtonHTMLAttributes<HTMLButtonElement>, HTMLButtonElement>, (value: string, index: number, array: readonly string[]) => unknown>, object>, "as">'.
  Overload 2 of 2, '(props: Substitute<DetailedHTMLProps<ButtonHTMLAttributes<HTMLButtonElement>, HTMLButtonElement>, (value: string, index: number, array: readonly string[]) => unknown>): ReactNode', gave the following error.
    Type '{ children: string; }' is not assignable to type '(value: string, index: number, array: readonly string[]) => unknown'.
      Type '{ children: string; }' provides no match for the signature '(value: string, index: number, array: readonly string[]): unknown'.ts(2769)
index.d.ts(3147, 9): 'name' is declared here.

Testing further, direct string interpolation does not work (is this not implemented yet or a deliberate decision?). But it seems I can pass a function that returns a string:

const ButtonWithStringInterpolation = styled.button`
  ${(): string => borderString}
  color: red;
`;

This works fine again (all styles applied) and raises no TS errors.

When I mix functions that return an object + string though:

const ButtonWithMixedInterpolation = styled.button`
  ${(): CSSProperties => borderRadiusObject}
  ${(): string => borderString}
  color: red;
`;

This works (all styles are applied) but raises a TypeScript error:

No overload matches this call.
  Overload 1 of 2, '(styles: TemplateStringsArray, ...args: ((options: ThemeArgs) => Primitve$2 | ComponentClass<{}, any>)[]): StyledComponent<...> & object', gave the following error.
    Type 'CSSProperties' is not assignable to type 'Primitve$2 | ComponentClass<{}, any>'.
  Overload 2 of 2, '(...styles: StyledArgument<ClassAttributes<HTMLButtonElement> & ButtonHTMLAttributes<HTMLButtonElement> & ((value: string, index: number, array: readonly string[]) => unknown)>[]): StyledComponent<...> & object', gave the following error.
    Argument of type '() => string' is not assignable to parameter of type 'StyledArgument<ClassAttributes<HTMLButtonElement> & ButtonHTMLAttributes<HTMLButtonElement> & ((value: string, index: number, array: readonly string[]) => unknown)>'.
      Type '() => string' is not assignable to type 'StyledCallback<ClassAttributes<HTMLButtonElement> & ButtonHTMLAttributes<HTMLButtonElement> & ((value: string, index: number, array: readonly string[]) => unknown)>'.
        Type 'string' is not assignable to type 'StyledCssArgument<ClassAttributes<HTMLButtonElement> & ButtonHTMLAttributes<HTMLButtonElement> & ((value: string, index: number, array: readonly string[]) => unknown)>'.ts(2769)
index.d.ts(63, 20): The expected type comes from the return type of this signature.

It seems Pigment does support the cases above (except for the direct ${string} interpolation). So only the types are a bit off, or am I mistaken?

Here's a minimal Next 14 app with latest Pigment:
https://github.com/molily/next14-pigment

Here's the file with the test code:
https://github.com/molily/next14-pigment/blob/main/src/app/PigmentOverview.tsx

Current behavior

Interpolation of different objects/functions into tagged template literal may cause a TypeScript error. One error disappears in the newest TypeScript version, the other remains.

Expected behavior

It should be clear which arguments can be passed to the tagged template literal, and types should reflect these capabilities.

Context

Next app with Styled Components adopting Pigment CSS

Your environment

Node v20.14.0
next 14.2.5
@pigment-css/nextjs-plugin 0.0.18
@pigment-css/react 0.0.18
typescript 5.3.3

Search keywords: tagged template literal, functions, TypeScript, Styled components migration
@molily molily added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jul 30, 2024
@zannager zannager added the package: system Specific to @mui/system label Jul 31, 2024
@FourwingsY
Copy link

FourwingsY commented Dec 19, 2024 •
edited
Loading

still happens in v0.0.28, and Typescript 5.7
BTW, I'm curious about the official stance on template literals. It was one of the main reasons I opted for pigmentcss from styled-components, (cause it makes migration a lot easier) but I couldn't find documentation about this functionality.

@brijeshb42
Copy link
Contributor

@molily At the moment, the typing decisions are deliberate. But if you see something that can be improved, we are open to contributions to expand the TS support as much as possible.

@FourwingsY It is supported will always be supported. The docs don't use it because it's not flexible when it comes to implementing variants. Stable version (v1) will encourage using it wherever possible.

@brijeshb42 brijeshb42 added this to the Road to v1 milestone Dec 20, 2024
@FourwingsY FourwingsY linked a pull request Dec 21, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@brijeshb42 brijeshb42

Labels
package: system Specific to @mui/system status: waiting for maintainer These issues haven't been looked at yet by a maintainer
Projects
None yet
Milestone
Road to v1
Development

Successfully merging a pull request may close this issue.

fix: type CreateStyledComponent to accept string and object FourwingsY/pigment-css
4 participants
@molily @brijeshb42 @FourwingsY @zannager