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

fix(typings): allow to have primitives as children for built-in #369

Closed
wants to merge 1 commit into from
Closed

fix(typings): allow to have primitives as children for built-in #369

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 18 additions & 21 deletions src/__tests__/__snapshots__/typescript.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,44 +75,41 @@ test/should-fail.test.tsx(184,15): error TS2551: Property 'colors' does not exis
test/should-fail.test.tsx(191,35): error TS2345: Argument of type 'StatelessComponent<ShouldClassNameUpdateProps>' is not assignable to parameter of type '\\"tspan\\"'.
test/should-fail.test.tsx(207,17): error TS2551: Property 'colors' does not exist on type 'ShouldClassNameUpdateContext'. Did you mean 'color'?
test/should-fail.test.tsx(217,11): error TS2345: Argument of type '\\"div\\"' is not assignable to parameter of type '\\"tspan\\"'.
test/should-fail.test.tsx(224,3): error TS2345: Argument of type '(props: { visible: boolean; }) => { primaryColor: boolean; }' is not assignable to parameter of type 'StyleArgument<CSSProperties, { visible: boolean; }>'.
Type '(props: { visible: boolean; }) => { primaryColor: boolean; }' is not assignable to type '(string | CSSProperties | StyleFunction<CSSProperties, { visible: boolean; }>)[]'.
Property 'push' is missing in type '(props: { visible: boolean; }) => { primaryColor: boolean; }'.
test/should-fail.test.tsx(229,1): error TS2554: Expected 1 arguments, but got 0.
test/should-fail.test.tsx(230,30): error TS2345: Argument of type '\\"\\"' is not assignable to parameter of type 'object'.
test/should-fail.test.tsx(231,30): error TS2345: Argument of type 'false' is not assignable to parameter of type 'object'.
test/should-fail.test.tsx(257,19): error TS2559: Type '{ d: \\"\\"; }' has no properties in common with type 'IntrinsicAttributes & IntrinsicClassAttributes<Component<HTMLProps<HTMLDivElement> & Partial<{ pr...'.
test/should-fail.test.tsx(258,19): error TS2322: Type '{ primaryColor: 1; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<Component<HTMLProps<HTMLDivElement> & Partial<{ pr...'.
test/should-fail.test.tsx(221,1): error TS2554: Expected 1 arguments, but got 0.
test/should-fail.test.tsx(222,30): error TS2345: Argument of type '\\"\\"' is not assignable to parameter of type 'object'.
test/should-fail.test.tsx(223,30): error TS2345: Argument of type 'false' is not assignable to parameter of type 'object'.
test/should-fail.test.tsx(249,19): error TS2559: Type '{ d: \\"\\"; }' has no properties in common with type 'IntrinsicAttributes & IntrinsicClassAttributes<Component<HTMLProps<HTMLDivElement> & Partial<{ pr...'.
test/should-fail.test.tsx(250,19): error TS2322: Type '{ primaryColor: 1; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<Component<HTMLProps<HTMLDivElement> & Partial<{ pr...'.
Type '{ primaryColor: 1; }' is not assignable to type 'Readonly<HTMLProps<HTMLDivElement> & Partial<{ primaryColor: string; }> & Pick<{ theme?: any; }, ...'.
Types of property 'primaryColor' are incompatible.
Type '1' is not assignable to type 'string | undefined'.
test/should-fail.test.tsx(259,31): error TS2559: Type '{ d: \\"\\"; }' has no properties in common with type 'IntrinsicAttributes & IntrinsicClassAttributes<Component<Partial<{ primaryColor: string; }> & Pic...'.
test/should-fail.test.tsx(260,31): error TS2322: Type '{ primaryColor: 1; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<Component<Partial<{ primaryColor: string; }> & Pic...'.
test/should-fail.test.tsx(251,31): error TS2559: Type '{ d: \\"\\"; }' has no properties in common with type 'IntrinsicAttributes & IntrinsicClassAttributes<Component<Partial<{ primaryColor: string; }> & Pic...'.
test/should-fail.test.tsx(252,31): error TS2322: Type '{ primaryColor: 1; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<Component<Partial<{ primaryColor: string; }> & Pic...'.
Type '{ primaryColor: 1; }' is not assignable to type 'Readonly<Partial<{ primaryColor: string; }> & Pick<{ theme: any; }, never> & ExtraGlamorousProps>'.
Types of property 'primaryColor' are incompatible.
Type '1' is not assignable to type 'string | undefined'.
test/should-fail.test.tsx(261,31): error TS2559: Type '{ d: \\"\\"; }' has no properties in common with type 'IntrinsicAttributes & IntrinsicClassAttributes<Component<object & Partial<{ primaryColor: string;...'.
test/should-fail.test.tsx(262,31): error TS2322: Type '{ primaryColor: 1; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<Component<object & Partial<{ primaryColor: string;...'.
test/should-fail.test.tsx(253,31): error TS2559: Type '{ d: \\"\\"; }' has no properties in common with type 'IntrinsicAttributes & IntrinsicClassAttributes<Component<object & Partial<{ primaryColor: string;...'.
test/should-fail.test.tsx(254,31): error TS2322: Type '{ primaryColor: 1; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<Component<object & Partial<{ primaryColor: string;...'.
Type '{ primaryColor: 1; }' is not assignable to type 'Readonly<object & Partial<{ primaryColor: string; }> & ExtraGlamorousProps>'.
Types of property 'primaryColor' are incompatible.
Type '1' is not assignable to type 'string | undefined'.
test/should-fail.test.tsx(267,15): error TS2345: Argument of type '{ textAlign: \\"center\\"; display: (\\"block\\" | \\"flexs\\")[]; }' is not assignable to parameter of type 'StyleArgument<CSSProperties, {}>'.
test/should-fail.test.tsx(259,15): error TS2345: Argument of type '{ textAlign: \\"center\\"; display: (\\"block\\" | \\"flexs\\")[]; }' is not assignable to parameter of type 'StyleArgument<CSSProperties, {}>'.
Type '{ textAlign: \\"center\\"; display: (\\"block\\" | \\"flexs\\")[]; }' is not assignable to type '(string | CSSProperties | StyleFunction<CSSProperties, {}>)[]'.
Property 'length' is missing in type '{ textAlign: \\"center\\"; display: (\\"block\\" | \\"flexs\\")[]; }'.
test/should-fail.test.tsx(272,18): error TS2345: Argument of type '{ textAlign: string; display: (\\"block\\" | \\"flexs\\")[]; }' is not assignable to parameter of type 'StyleArgument<SVGProperties, {}>'.
test/should-fail.test.tsx(264,18): error TS2345: Argument of type '{ textAlign: string; display: (\\"block\\" | \\"flexs\\")[]; }' is not assignable to parameter of type 'StyleArgument<SVGProperties, {}>'.
Type '{ textAlign: string; display: (\\"block\\" | \\"flexs\\")[]; }' is not assignable to type '(string | SVGProperties | StyleFunction<SVGProperties, {}>)[]'.
Property 'length' is missing in type '{ textAlign: string; display: (\\"block\\" | \\"flexs\\")[]; }'.
test/should-fail.test.tsx(289,35): error TS2322: Type '{ display: \\"blocks\\"; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<Component<object & CSSProperties & ExtraGlamorousP...'.
Property 'length' is missing in type '{ textAlign: "center"; display: (\\"block\\" | \\"flexs\\")[]; }'.
test/should-fail.test.tsx(281,35): error TS2322: Type '{ display: \\"blocks\\"; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<Component<object & CSSProperties & ExtraGlamorousP...'.
Type '{ display: \\"blocks\\"; }' is not assignable to type 'Readonly<object & CSSProperties & ExtraGlamorousProps>'.
Types of property 'display' are incompatible.
Type '\\"blocks\\"' is not assignable to type '\\"none\\" | \\"table\\" | \\"ruby\\" | \\"initial\\" | \\"inherit\\" | \\"unset\\" | \\"block\\" | \\"inline\\" | \\"run-in\\" | \\"fl...'.
test/should-fail.test.tsx(290,38): error TS2559: Type '{ display: \\"block\\"; }' has no properties in common with type 'IntrinsicAttributes & IntrinsicClassAttributes<Component<object & ExtraGlamorousProps, ComponentS...'.
test/should-fail.test.tsx(291,42): error TS2559: Type '{ display: \\"block\\"; }' has no properties in common with type 'IntrinsicAttributes & IntrinsicClassAttributes<Component<object & ExtraGlamorousProps, ComponentS...'.
test/should-fail.test.tsx(293,29): error TS2322: Type '{ display: \\"blocks\\"; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<Component<HTMLProps<HTMLDivElement> & object & CSS...'.
test/should-fail.test.tsx(282,38): error TS2559: Type '{ display: \\"block\\"; }' has no properties in common with type 'IntrinsicAttributes & IntrinsicClassAttributes<Component<object & ExtraGlamorousProps, ComponentS...'.
test/should-fail.test.tsx(283,42): error TS2559: Type '{ display: \\"block\\"; }' has no properties in common with type 'IntrinsicAttributes & IntrinsicClassAttributes<Component<object & ExtraGlamorousProps, ComponentS...'.
test/should-fail.test.tsx(285,29): error TS2322: Type '{ display: \\"blocks\\"; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<Component<HTMLProps<HTMLDivElement> & object & CSS...'.
Type '{ display: \\"blocks\\"; }' is not assignable to type 'Readonly<HTMLProps<HTMLDivElement> & object & CSSProperties & ExtraGlamorousProps>'.
Types of property 'display' are incompatible.
Type '\\"blocks\\"' is not assignable to type '\\"none\\" | \\"table\\" | \\"ruby\\" | \\"initial\\" | \\"inherit\\" | \\"unset\\" | \\"block\\" | \\"inline\\" | \\"run-in\\" | \\"fl...'.
test/should-fail.test.tsx(294,32): error TS2559: Type '{ display: \\"block\\"; }' has no properties in common with type 'IntrinsicAttributes & IntrinsicClassAttributes<Component<HTMLProps<HTMLDivElement> & object & Ext...'.
test/should-fail.test.tsx(295,36): error TS2559: Type '{ display: \\"block\\"; }' has no properties in common with type 'IntrinsicAttributes & IntrinsicClassAttributes<Component<HTMLProps<HTMLDivElement> & object & Ext...'.
test/should-fail.test.tsx(286,32): error TS2559: Type '{ display: \\"block\\"; }' has no properties in common with type 'IntrinsicAttributes & IntrinsicClassAttributes<Component<HTMLProps<HTMLDivElement> & object & Ext...'.
test/should-fail.test.tsx(287,36): error TS2559: Type '{ display: \\"block\\"; }' has no properties in common with type 'IntrinsicAttributes & IntrinsicClassAttributes<Component<HTMLProps<HTMLDivElement> & object & Ext...'.
"
`;
16 changes: 16 additions & 0 deletions test/glamorous.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -564,3 +564,19 @@ const BuiltinStyledWithMultipleChild: JSX.Element = (
<glamorous.Span>world!</glamorous.Span>
</glamorous.Div>
);

const BuiltinStyledWithPrimitivesChildren: JSX.Element = (
<glamorous.Div color='red'>
{null}
{false}
{true}
{undefined}
{5}
</glamorous.Div>
);

const BuiltinStyledWithFragment: JSX.Element = (
<glamorous.Div color='red'>
<React.Fragment />
</glamorous.Div>
);
8 changes: 0 additions & 8 deletions test/should-fail.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,14 +218,6 @@ glamorous('div', {
withProps: ''
})

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm assuming this was removed by accident as it looks like it should fail and appears unrelated to the changes. Would you mind re adding if that's the case thanks.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is removed on purpose, as this relaxation is the side-effect of the proposed change.

The new indexed type allows the value to be boolean, which makes the primaryColor: props.visible be valid.

Of course, it’s silly that we have this effect, but as noted in the other comment, it’d be awesome to not even have children in the CSSProperties. Until then this is a workaround.

The PR is effectively relaxing the type-checking for this instance, meaning fewer errors will be caught by TypeScript compiler. However, requirements that are too lax are much better than requirements that are too strict. In the latter case the developers fall into habit into silencing errors (with as any), defeating the entire purpose of typechecking in the first place.

glamorous('div', {
withProps: { visible: false }
})(
(props) => ({
primaryColor: props.visible
})
)

glamorous('div')().withProps()
glamorous('div')().withProps('')
glamorous('div')().withProps(false)
Expand Down
2 changes: 1 addition & 1 deletion typings/css-properties.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1926,7 +1926,7 @@ export interface CSSPropertiesLossy {
| undefined
| Array<CSSPropertiesComplete[keyof CSSPropertiesComplete]>
| CSSPropertiesLossy
| React.ReactChild
| React.ReactNode
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update, I'd love to remove this entirely from this file as a ReactNode/ReactChild is not a css property.

But keeping things working in the meantime sounds great 💯.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Me too, but small steps at a time. :-(

I have tried investigating the root cause, but I can not understand why does children match the index signature on CSSProperties rather than the existing children on React components. I tried overriding children even directly in CSSPropertiesLossy, but the index still takes priority. By the way, if I remove CSSPropertiesLossy entirely, my code typechecks. But it, of course, might not be the solution.

}

export interface CSSProperties
Expand Down
2 changes: 1 addition & 1 deletion typings/svg-properties.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ export interface SVGPropertiesLossy {
| undefined
| Array<SVGPropertiesCompleteSingle[keyof SVGPropertiesCompleteSingle]>
| SVGPropertiesLossy
| React.ReactChild
| React.ReactNode
}

export interface SVGProperties
Expand Down