Skip to content
This repository has been archived by the owner on Dec 13, 2018. It is now read-only.

typescript: built-in components don't accept Components as children #304

Closed
luke-john opened this issue Aug 24, 2017 · 7 comments
Closed

Comments

@luke-john
Copy link
Collaborator

Originally reported by @Kais-DkM at #303.

<glamorous.Div>
  <glamorous.Span>Hello, world!</glamorous.Span>
</glamorous.Div>

What you did:

Used a Component as a child of a built in glamorous component

What happened:

Typescript errored with Type '{ children: Element; }' is not assignable to type 'CSSPropertiesDom'.

Problem description:

The index type for CSSPropertiesLossy is too strict for glamorous component factories

Suggested solution:

Use a custom version of CSSProperties with built in component factories along the following lines

// css-properties.d.ts
export interface CSSPropertiesLossyDom {
  [propertyName: string]:
    | string | number | CSSPropertiesComplete | undefined
    | Array<CSSPropertiesComplete[keyof CSSPropertiesComplete]>
    | CSSPropertiesLossy
    | React.ReactNode
}

export interface CSSPropertiesDom extends
  CSSPropertiesLossyDom,
  CSSPropertiesComplete,
  CSSPropertiesPseudo
    {}

// /built-in-glamorous-components.d.ts
  A: React.StatelessComponent<CSSPropertiesDom & ExtraGlamorousProps & React.HTMLProps<HTMLAnchorElement>>

Other Notes:

For anyone tackling this issue it would be great to add some tests for the built in component factories.

@remagpie
Copy link
Contributor

remagpie commented Aug 24, 2017

To add more information, currently only built-in components with single JSX child failes to pass type checking.
For example, as posted on original post,

<glamorous.Div>
  <glamorous.Span>Hello, world!</glamorous.Span>
</glamorous.Div>

fails with error

Type '{ children: Element; }' is not assignable to type 'IntrinsicAttributes & CSSProperties & ExtraGlamorousProps & HTMLProps<HTMLDivElement> & { childre...'.
Type '{ children: Element; }' is not assignable to type 'CSSProperties'.
Property 'children' is incompatible with index signature.
Type 'Element' is not assignable to type 'string | number | SingleOrArray<CSSPropertiesCompleteSingle, "mask" | "alignContent" | "alignItem...'.
Type 'Element' is not assignable to type '(string | number | {} | (string | number | undefined)[] | ("initial" | "inherit" | "unset" | "fle...'.
Property 'length' is missing in type 'Element'.

However, all of the following works without error:

<glamorous.Div/>
<glamorous.Div>
  Hello, world!
</glamorous.Div>
<glamorous.Div>
  <glamorous.Span>Hello,</glamorous.Span>
  <glamorous.Span>world!</glamorous.Span>
</glamorous.Div>

Personally, I suspect that there's something more going on other than strictness of CSSPropertiesLossy, but I can't figure out what it is....

@luke-john
Copy link
Collaborator Author

<glamorous.Div>
  <glamorous.Span>Hello,</glamorous.Span>
  <glamorous.Span>world!</glamorous.Span>
</glamorous.Div>

I suspect this passes currently because;

  1. CSSPropertiesLossy accepts an array of CSSProperties
  2. all the properties on CSSProperties are optional,
  3. empty interfaces pass for pretty much everything in typescript.

@remagpie
Copy link
Contributor

@luke-john
I think 1 and 2 are right, but I'm not quite sure if 3 is related to this issue.
For following code:

interface Foo {
  foo?: number;
}
const x: Foo = { bar: 'asdf' };
const y: {} = { bar: 'asdf' };

y doesn't give error, but x gives following error:

Type '{ bar: string; }' is not assignable to type 'Foo'.
Object literal may only specify known properties, and 'bar' does not exist in type 'Foo'.

I think 3 doesn't apply to partial type, like CSSProperties for our case.

@luke-john
Copy link
Collaborator Author

Im on mobile till tmo, but the following should demonstrate the issue with
interfaces with only optionals accepting other things.

interface Foo { foo?: number; }

const x: Foo = () => 'passes';
const y: {} = { bar: 'asdf' };

Edit: redid comment as email replies do not support markdown

@Antontelesh
Copy link

Getting the same error now:
2018-01-29 13 47 12

TS2322: Type '{ css: { flexGrow: 1; }; children: ReactNode; }' is not assignable to type 'IntrinsicAttributes & CSSProperties & ExtraGlamorousProps & HTMLProps<HTMLDivElement> & { childre...'.
  Type '{ css: { flexGrow: 1; }; children: ReactNode; }' is not assignable to type 'CSSProperties'.
    Property 'children' is incompatible with index signature.
      Type 'ReactNode' is not assignable to type 'string | number | SingleOrArray<CSSPropertiesCompleteSingle, "mask" | "alignContent" | "alignItem...'.
        Type 'true' is not assignable to type 'string | number | SingleOrArray<CSSPropertiesCompleteSingle, "mask" | "alignContent" | "alignItem...'.

TypeScript says that this.props.children has type React.ReactNode.

glamor: v2.20.40
glamorous: v4.11.4
typescript: v2.6.2

@deep-c
Copy link

deep-c commented Feb 11, 2018

I am experiencing the same problem as @Antontelesh glamorous@4.11.4, typescript@2.7.1, glamor@2.20.40 with the following:

<Div css={{ ...theme.commentWrapper }}>
    {children}
</Div>

@luke-john

@Ailrun
Copy link
Contributor

Ailrun commented Feb 11, 2018

@Antontelesh @deep-c Currently, we are trying to remove React.ReactChild from CSSProperties and solve #353.
See #372.
I believe this PR will solve your problem too.

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

No branches or pull requests

5 participants