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

TS error when using prop with ThemedStyledComponentsModule #53

Open
DevanB opened this issue Nov 2, 2018 · 11 comments
Open

TS error when using prop with ThemedStyledComponentsModule #53

DevanB opened this issue Nov 2, 2018 · 11 comments

Comments

@DevanB
Copy link

DevanB commented Nov 2, 2018

I'm trying to use prop within ifProp, but getting an error that:

[ts]
Argument of type '<Props>(props?: Props) => Props[keyof Props]' is not assignable to parameter of type 'Interpolation<ThemeProps<ThemeInterface>>'.
  Type '<Props>(props?: Props) => Props[keyof Props]' is not assignable to type 'InterpolationFunction<ThemeProps<ThemeInterface>>'.
    Type 'ThemeInterface' is not assignable to type 'Interpolation<ThemeProps<ThemeInterface>>'.
      Type 'ThemeInterface' is not assignable to type 'ReadonlyArray<string | number | false | Styles | Keyframes | StyledComponentClass<any, any, any> | InterpolationFunction<ThemeProps<ThemeInterface>> | ReadonlyArray<FlattenInterpolation<ThemeProps<ThemeInterface>>>>'.
        Property 'length' is missing in type 'ThemeInterface'. [2345]
interface SpaceProps {
  around?: string;
  bottom?: string;
  left?: string;
  right?: string;
  top?: string;
}

export const Space = styled.div<SpaceProps>`
  ${ifProp(
    'around',
    css`
      margin: ${prop('around')}em;
    `
  )};
  ${ifProp(
    'top',
    css`
      margin-top: ${prop('top')}em;
    `
  )};
  ${ifProp(
    'right',
    css`
      margin-right: ${prop('right')}em;
    `
  )};
  ${ifProp(
    'bottom',
    css`
      margin-bottom: ${prop('bottom')}em;
    `
  )};
  ${ifProp(
    'left',
    css`
      margin-left: ${prop('left')}em;
    `
  )};
`;

I'm assuming I'm doing something wrong, but can't quite pinpoint what the problem may be.

@DevanB
Copy link
Author

DevanB commented Nov 11, 2018

Curious if anyone has had this problem. I still haven't figured this out, yet.

@diegohaz
Copy link
Owner

Sorry @DevanB. I missed this issue. The code you pasted seems right. I've never seen that error with styled-tools. If you can provide a small reproduction repository, I can investigate it further.

@DevanB
Copy link
Author

DevanB commented Nov 14, 2018

I've been trying to reproduce it in CodeSandbox, but haven't had much luck yet.

The problem only occurs when I update to styled-components v4, for some reason.

I'll keep trying to reproduce.

UPDATE:

The issue is no longer an issue if I remove the ifProps, but still use the prop utility. The issue also goes away if I remove the prop usage and hard code a value like 1em

UPDATE AGAIN:

I was able to fix it completely by doing this:

import { ifProp, withProp } from 'styled-tools';
import { css, styled } from '../theme';

interface SpaceProps {
  around?: number;
  bottom?: number;
  left?: number;
  right?: number;
  top?: number;
}

export const Space = styled.div<SpaceProps>`
  ${ifProp(
    'around',
    css`
      margin: ${withProp('around', around => `${around}em`)};
    `
  )}
  ${ifProp(
    'bottom',
    css`
      margin-bottom: ${withProp('bottom', bottom => `${bottom}em`)};
    `
  )}
  ${ifProp(
    'left',
    css`
      margin-left: ${withProp('left', left => `${left}em`)};
    `
  )}
  ${ifProp(
    'right',
    css`
      margin-right: ${withProp('right', right => `${right}em`)};
    `
  )}
  ${ifProp(
    'top',
    css`
      margin-top: ${withProp('top', top => `${top}em`)};
    `
  )}
`;

@DevanB
Copy link
Author

DevanB commented Nov 14, 2018

@diegohaz Okay, so the above works, but I think I've found a logical error.

When I use the component like <Space around={3} right={0}/>, I get:

margin: 3em;

When I expect it to be:

margin: 3em;
margin-right: 0em;
${ifProp(
  'right',
  css`
    margin-right: ${withProp('right', right => `${right}em`)};
  `
)}

is evaluating to false, because 0 is a falsey value in JavaScript (don't we love it 🙄 ).

Is there a way to treat 0 as a value, not a true/false evaluation? Or would that require a source code change to do some sort of coercion/triple equality check?

@diegohaz
Copy link
Owner

You can pass a function:

ifProp(props => props.right !== undefined, true, false);

By the way, only in your last update you put this line:

import { css, styled } from '../theme';

I guess the typing problem with prop (which is definitely the one you should use, and not withProp) might come from some implementation in ../theme.

@DevanB
Copy link
Author

DevanB commented Nov 14, 2018

I gathered something in our theme file was the issue. In it we are extending styled-components to use our ThemeInterface. Perhaps we aren't exporting something we should be exporting. This was happening at one point: we were using your awesome theme() utility but not exporting our ThemeInterface.

import * as styledComponents from 'styled-components';
import { createGlobalStyle } from 'styled-components';
import { globalStyle } from './globalStyle';

export interface ThemeInterface {
  bezier: string;
  black: string;
  blue: string;
  body0: string;
  body1: string;
  body2: string;
  body3: string;
  corner: string;
  darkGray0: string;
  darkGray1: string;
  error: string;
  green: string;
  heading0: string;
  heading1: string;
  heading2: string;
  ink: string;
  lightGray0: string;
  lightGray1: string;
  lightGray2: string;
  mobile: string;
  pink: string;
  primary: string;
  primaryLight: string;
  red: string;
  sans: string;
  secondary: string;
  serif: string;
  tablet: string;
  white: string;
  yellow: string;
}

export const Theme = {
  // bezier: 'cubic-bezier(0.51, 0.28, 0.46, 0.93)',
  bezier: 'cubic-bezier(0.23, 1, 0.32, 1)',
  black: '#222222',
  blue: '#294998',
  body0: '0.875em', // 14px
  body1: '1em', // 16px
  body2: '1.125em', // 18px
  body3: '1.25em', // 20px
  corner: '0.3125em',
  darkGray0: '#3e3e3e',
  darkGray1: '#676767',
  error: '#d83479',
  green: '#2c9360',
  heading0: '1.5625em', // 25px
  heading1: '2.1875em', // 35px
  heading2: '2.5em', // 40px
  ink: '#00004f',
  lightGray0: '#f5f5f5',
  lightGray1: '#ebebeb',
  lightGray2: '#d2d2d2',
  mobile: '31.25em',
  pink: '#e1a0ad',
  primary: '#19678d',
  primaryLight: '#e2e9ed',
  red: '#ce3c36',
  sans: '"Roboto", sans-serif',
  secondary: '#229bd5',
  serif: '"Merriweather", serif',
  tablet: '48em',
  white: '#ffffff',
  yellow: '#ebb23f',
};

const GlobalStyle = createGlobalStyle`${globalStyle}`;

const {
  default: styled,
  css,
  keyframes,
  ThemeProvider,
} = styledComponents as styledComponents.ThemedStyledComponentsModule<
  ThemeInterface
>;

export { css, GlobalStyle, keyframes, styled, ThemeProvider };

@diegohaz
Copy link
Owner

diegohaz commented Nov 14, 2018

If you import css directly from styled-components it doesn't show any errors, right? There should be some inconsistency between styled-tools types and the way you re-create styled-components members with your theme.

From what I see, you're doing it right, so probably it's a styled-tools issue. I have no idea how to fix this though.


Ah, not directly related to the issue, but I think you don't need to define the theme shape twice (export interface ThemeInterface and export const Theme). You can just use the type of your object (not sure if it works though):

export const theme = {
  ...
};

export type ThemeInterface = typeof theme;

@DevanB
Copy link
Author

DevanB commented Nov 14, 2018

The good news is that it is working for us (thanks for your consistent and continual help!). I'll keep an eye on it and see what could possibly be the issue.

Would you want to leave this open as an issue, or should I submit an issue with more specific details and close this issue?

@diegohaz
Copy link
Owner

I think we should keep this open. I'll update the title to better reflect the problem.

@diegohaz diegohaz changed the title Using prop within ifProp TS error when using prop with ThemedStyledComponentsModule Nov 14, 2018
@DevanB
Copy link
Author

DevanB commented Nov 14, 2018

Think I found another instance of something not agreeing with TypeScript and/or ThemeStyledComponentsModule:

src/Buttons.tsx:17:19 - error TS2345: Argument of type '<Props, Theme extends { [key: string]: any; }>(props: Props& { theme: Theme; }) => Theme[keyof Theme] | undefined' is not assignable to parameter of type 'string | ((props?: { theme: { [key: string]: any; }; } | undefined) => any) | Needle<{ theme: { [key: string]: any; }; }>[]'.
  Type '<Props, Theme extends { [key: string]: any; }>(props: Props & { theme: Theme; }) => Theme[keyof Theme] | undefined' is not assignable to type '(props?: { theme: { [key: string]: any; }; } | undefined) => any'.
    Types of parameters 'props' and 'props' are incompatible.
      Type '{ theme: { [key: string]: any; }; } | undefined' is not assignable to type '{ theme: { [key: string]: any; }; }'.
        Type 'undefined' is not assignable to type '{ theme: { [key: string]: any; }; }'.

17   green: withProp(theme('green'), darken(0.1)),

In here, it seems the types of styled-tools and possibly of styled-components aren't aligned. I'll keep looking.

@diegohaz
Copy link
Owner

If you go to node_modules/styled-tools/types/index.d.ts, you can tweak the typings and see the results in your app. This would be only for debugging purposes, of course, we should implement the fixes here in the project.

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

No branches or pull requests

2 participants