Emotion: TypeScript props and HTML attributes collisions

[kshahani]

• emotion version: 10.0.0
• react version: ^16

Relevant code:

type Props = {
  color: string | string [];
}

const MyComponent = styled('div')<Props>`
  color: pink;
`;

What you did:

I want a prop that coincides with the name of an attribute on the HTMLElement type (i.e.: color exists on a div element as a type string, but I want a prop called color which could be either a string or a string[]). For a little bit of more context, this is how styled-system works with its exported style prop color and type ColorProps.

What happened:

The relevant debugging information is Type 'string | string[]' is not assignable to type 'string'.

Reproduction:

Simply copy the code snippet above into your editor.

Problem description:

I'm rendered unable to type this component without renaming that prop, which I really don't want to do because that breaks the styled-system contract.

Suggested solution:

No ideas! Any help welcome.

[travisbloom]

I'm struggling with this as well. Trying to find if there's a way of omitting some of the default html props so we can override.

[kshahani]

Our workaround is:

const MyComponent = styled<'div', Props>('div')`
  color: pink;
`;

...which allows the ExtraProps to override the attributes on a div element.

[maximtmartin]

@kshahani are you sure this works?

When I try this I'm still getting typescript errors:

type ColorProps = string | Partial<Record<Breakpoint, string> & { _: string; }>;
type ButtonProps = ColorProps

const Button = styled<'button', ButtonProps>('button', { shouldForwardProp: prop => isPropValid(prop) || prop === 'routed' })(...)

usage:

<Button color={{ _: 'green.2' }} > Test </Button>

Ts error:

Type '{ _: string; }' is not assignable to type 'string | (string & Partial & { _: string; }>) | undefined'.
Type '{ _: string; }' is not assignable to type 'string & Partial & { _: string; }>'.
Type '{ _: string; }' is not assignable to type 'string'.

23 color={{ _: 'green.2' }}

node_modules/@types/react/index.d.ts:1682:9
1682 color?: string;
~
The expected type comes from property 'color' which is declared here on type 'IntrinsicAttributes & ClassAttributes & ButtonHTMLAttributes & Pick & ButtonHTMLAttributes & ... 7 more ... & { ...; }, "onAbort" | ... 307 more ... | "as"> & { ...; } & { ...; }'

[shulcsm]

Our workaround is:

const MyComponent = styled<'div', Props>('div')`
  color: pink;
`;

...which allows the ExtraProps to override the attributes on a div element.

This does not seem to work at least in v10. Seems like intrinsic attributes take precedence over ExtraProps.

Something like this, works. But it's really ugly.

export const Box = styled('div', { shouldForwardProp })(
  { boxSizing: 'border-box' },
  space,
  color,
  width,
  fontSize,
  flex,
  alignSelf,
) as React.ComponentType<Omit<React.HTMLAttributes<HTMLDivElement>, keyof BoxComponentProps> & BoxComponentProps>;

[potty]

For me, the solution was to use Radix https://github.com/modulz/radix instead of styled-system directly. Radix uses some parts of styled-system and has better TS support.

[peduarte]

@potty Thanks! Glad it's solved your problem 👍

Just for everyone's clarity, Radix System is the wrapper on top of Styled System Core. Radix is the actual Design System for Modulz which is built on top of Radix System.

[Andarist]

From what I can tell the issue here was about types with styled-system - that's somewhat out of the scope of this repository.

[peduarte]

@Andarist That's not really true. It's not a Styled System issue, per se.

The issue is if you try type a prop which also happens to be a HTML Attribute.

For example, say you have an input component, and you want that to accept a size prop, and you want that to be of type array of strings or numbers (don't ask why 😛), then you get this type checking conflict.

[Andarist]

and you want that to accept a size prop, and you want that to be of type array of strings or numbers

This is not a valid thing to do in emotion though (with default options). size will get passed onto underlying input element (in this example and unless you are using custom shouldForwardProp) which will result in an invalid value being set on as HTML attribute.

This indeed can be tweaked at runtime (with mentioned shouldForwardProp) - but supporting this at type level seems like bringing a lot of complexity into already complicated typings. TS doesn't have an easy way of overriding existing types. We'd have to accept Yet Another Type Parameter and use a helper like this:

type Merge<M, N> = Omit<M, Extract<keyof M, keyof N>> & N

to output correct type. This still wouldn't really be sound - because we wouldn't be (at least not easily) able to constraint configured shouldForwardProp to really follow what has been specified in those types.

It seems to me that using as assertion for this is the way to go - it's ugly but also warns a reader that this is some funky unsound stuff (which is a good thing). This ugliness can be hidden in one place in your codebase, so I would say that this is an acceptable solution.

@JakeGinnivan Please correct me if I'm talking bollocks here :p

[JakeGinnivan]

@Andarist created a PR with a potential solution. It's another overload so it shouldn't increase compile time for those not using the feature? That is untested though

[peduarte]

@Andarist yeah I totally get what you mean. In a way, it feels like we shouldn't be creating props that clash with DOM Attributes. And that is what Styled System does by default, for example, color in a p element.