Skip to content

Commit

Permalink
fix(typescript): Improved typings to prevent incorrect usage
Browse files Browse the repository at this point in the history
The usage of a union between Partials and Functions resulted in any functions passed to glamorous
component factories being accepted by the type system.  As part of fixing this I also renamed some
types to align with the glamorous documentation.

BREAKING CHANGE: Renamed types `HTMLGlamorousInterface` -> `HTMLComponentFactory`, `StyledFunction`
-> `GlamorousComponentFactory`, `SVGGlamorousInterface` -> `SVGComponentFactory`. Also improved
typesafety on them, so things that previously could have passed will now fail.

212
  • Loading branch information
luke-john committed Jul 10, 2017
1 parent 1564a75 commit f949dde
Show file tree
Hide file tree
Showing 13 changed files with 654 additions and 224 deletions.
3 changes: 1 addition & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
"start": "nps",
"doctoc": "doctoc --maxlevel 3 --title \"**Table of Contents**\"",
"test": "nps test",
"test:ts": "(tsc -p ./tsconfig.json && rimraf test-ts) || rimraf test-ts",
"commitmsg": "opt --in commit-msg --exec \"validate-commit-msg\"",
"precommit": "lint-staged && opt --in pre-commit --exec \"npm start validate\""
},
Expand Down Expand Up @@ -83,7 +82,7 @@
"rollup-plugin-uglify": "^2.0.1",
"tslint": "^5.4.3",
"tslint-microsoft-contrib": "^5.0.0",
"typescript": "^2.3.4",
"typescript": "^2.4.1",
"validate-commit-msg": "^2.12.2"
},
"eslintConfig": {
Expand Down
26 changes: 26 additions & 0 deletions src/__tests__/__snapshots__/typescript.js.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Typescript expected failures 1`] = `
"yarn test:ts v0.24.6
$ (tsc -p ./tsconfig.json && rimraf test-ts) || rimraf test-ts
test/glamorous-should-fail.tsx(16,3): error TS2345: Argument of type '{ fillRule: \\"cat\\"; }' is not assignable to parameter of type 'Partial<SVGProperties>'.
Types of property 'fillRule' are incompatible.
Type '\\"cat\\"' is not assignable to type '\\"nonzero\\" | \\"evenodd\\" | \\"inherit\\" | undefined'.
test/glamorous-should-fail.tsx(29,3): error TS2345: Argument of type '() => { fillRule: \\"cat\\"; }' is not assignable to parameter of type 'Partial<SVGProperties>'.
Index signature is missing in type '() => { fillRule: \\"cat\\"; }'.
test/glamorous-should-fail.tsx(43,3): error TS2345: Argument of type '{ float: \\"cat\\"; }' is not assignable to parameter of type 'Partial<CSSProperties>'.
Types of property 'float' are incompatible.
Type '\\"cat\\"' is not assignable to type '\\"left\\" | \\"right\\" | \\"none\\" | \\"initial\\" | \\"inherit\\" | \\"unset\\" | \\"inline-start\\" | \\"inline-end\\" | und...'.
test/glamorous-should-fail.tsx(55,3): error TS2345: Argument of type '() => { float: \\"cat\\"; }' is not assignable to parameter of type 'Partial<CSSProperties>'.
Index signature is missing in type '() => { float: \\"cat\\"; }'.
test/glamorous-should-fail.tsx(71,3): error TS2345: Argument of type '{ fillRule: \\"cat\\"; }' is not assignable to parameter of type 'Partial<CSSProperties>'.
Types of property 'fillRule' are incompatible.
Type '\\"cat\\"' is not assignable to type '\\"nonzero\\" | \\"evenodd\\" | \\"initial\\" | \\"inherit\\" | \\"unset\\" | undefined'.
test/glamorous-should-fail.tsx(77,3): error TS2345: Argument of type '() => { fillRule: \\"cat\\"; }' is not assignable to parameter of type 'Partial<CSSProperties>'.
Index signature is missing in type '() => { fillRule: \\"cat\\"; }'.
test/glamorous-should-fail.tsx(83,3): error TS2345: Argument of type '{ float: \\"cat\\"; }' is not assignable to parameter of type 'Partial<CSSProperties>'.
Types of property 'float' are incompatible.
Type '\\"cat\\"' is not assignable to type '\\"left\\" | \\"right\\" | \\"none\\" | \\"initial\\" | \\"inherit\\" | \\"unset\\" | \\"inline-start\\" | \\"inline-end\\" | und...'.
test/glamorous-should-fail.tsx(89,3): error TS2345: Argument of type '() => { float: \\"cat\\"; }' is not assignable to parameter of type 'Partial<CSSProperties>'.
Index signature is missing in type '() => { float: \\"cat\\"; }'."
`;
18 changes: 18 additions & 0 deletions src/__tests__/typescript.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
var spawnSync = require('child_process').spawnSync

test('Typescript', () => {
const testTypescriptCompilation = spawnSync('yarn', ['test:ts'])

const typescriptCompilationOutput = testTypescriptCompilation.stdout
.toString()
.trimRight()

const trimmedTypescriptCompilationOutput = typescriptCompilationOutput.substring(
0,
typescriptCompilationOutput.lastIndexOf('\n'),
)

expect(trimmedTypescriptCompilationOutput).toMatchSnapshot(
'Typescript expected failures',
)
})
6 changes: 3 additions & 3 deletions test/glamorous-exports.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import {
CSSProperties,
ExtraGlamorousProps,
GlamorousComponent,
HTMLGlamorousInterface,
StyledFunction,
SVGGlamorousInterface,
HTMLComponentFactory,
GlamorousComponentFactory,
SVGComponentFactory,
} from '../'
92 changes: 92 additions & 0 deletions test/glamorous-should-fail.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
import * as React from "react";

import glamorous, { withTheme, ThemeProvider } from "../";

// built-in DOM component factories

// ### SVG

const BuiltInStrictSVGStyleObjectInvalidKey = glamorous.svg(
{
flying: 'cat',
},
)

const BuiltInStrictSVGStyleObjectInvalidProperty = glamorous.svg(
{
fillRule: 'cat',
},
)

// Known to fail currently due to typescripts partial handling
const BuiltInStrictSVGStyleFunctionInvalidKey = glamorous.svg(
() => ({
flying: 'cat'
})
)

const BuiltInStrictSVGStyleFunctionInvalidProperty = glamorous.svg(
() => ({
fillRule: 'cat',
})
)

// ### HTML

const BuiltInStrictDIVtyleObjectInvlalidKey = glamorous.div(
{
flying: "cat",
},
)

const BuiltInStrictDIVtyleObjectInvlalidProperty = glamorous.div(
{
float: "cat",
},
)

const BuiltInStrictDIVStyleFunctionInvlalidKey = glamorous.div(
() => ({
flying: "cat",
})
)

const BuiltInStrictDIVStyleFunctionInvlalidProperty = glamorous.div(
() => ({
float: "cat",
})
)

// self provided glamorousComponentFactory

interface TestComponentProps {
className: string
}

const TestComponent: React.SFC<TestComponentProps> = (props) => (
<div className={props.className} />
)

const StrictSVGStyleObject = glamorous(TestComponent)(
{
fillRule: 'cat',
},
)

const StrictSVGStyleFunction = glamorous(TestComponent)(
() => ({
fillRule: 'cat',
})
)

const BuiltInStrictStyleFunction = glamorous(TestComponent)(
{
float: "cat",
},
)

const BuiltInStrictDivStyleFunction = glamorous(TestComponent)(
() => ({
float: "cat",
})
)
7 changes: 5 additions & 2 deletions test/glamorous.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,15 @@ interface DividerInsideDividerProps {
color: string;
};

type DividerInsideDividerTheme = { main: { color: string; } }


// component styles
const DividerInsideDivider = glamorous(Divider)(
{
"fontSize": "10px",
fontSize: "10px",
},
(props, theme: { main: { color: string; } }) => ({
(props, theme: DividerInsideDividerTheme) => ({
"color": theme.main.color,
}),
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,24 @@
import { SVGAttributes } from 'react'

import { CSSProperties } from './css-properties'
import { SVGProperties } from './svg-properties'
import {
HtmlStyledFunction,
SvgStyledFunction,
} from './styled-function'
GlamorousComponentFactory,
} from './component-factory'

export type HtmlStyledFunction<
HTMLElement,
Properties
> = GlamorousComponentFactory<React.HTMLProps<HTMLElement>, Properties>


export interface HTMLGlamorousInterface {
export type SvgStyledFunction<
SVGElement,
Attributes
> = GlamorousComponentFactory<React.SVGAttributes<SVGElement>, Attributes>


export interface HTMLComponentFactory {
a: HtmlStyledFunction<HTMLAnchorElement, CSSProperties>
abbr: HtmlStyledFunction<HTMLElement, CSSProperties>
address: HtmlStyledFunction<HTMLElement, CSSProperties>
Expand Down Expand Up @@ -121,25 +134,24 @@ export interface HTMLGlamorousInterface {
wbr: HtmlStyledFunction<HTMLElement, CSSProperties>
}


export interface SVGGlamorousInterface {
circle: SvgStyledFunction<SVGCircleElement, SVGAttributes<any>>
clipPath: SvgStyledFunction<SVGClipPathElement, SVGAttributes<any>>
defs: SvgStyledFunction<SVGDefsElement, SVGAttributes<any>>
ellipse: SvgStyledFunction<SVGEllipseElement, SVGAttributes<any>>
g: SvgStyledFunction<SVGGElement, SVGAttributes<any>>
image: SvgStyledFunction<SVGImageElement, SVGAttributes<any>>
line: SvgStyledFunction<SVGLineElement, SVGAttributes<any>>
linearGradient: SvgStyledFunction<SVGLinearGradientElement, SVGAttributes<any>>
mask: SvgStyledFunction<SVGMaskElement, SVGAttributes<any>>
path: SvgStyledFunction<SVGPathElement, SVGAttributes<any>>
pattern: SvgStyledFunction<SVGPatternElement, SVGAttributes<any>>
polygon: SvgStyledFunction<SVGPolygonElement, SVGAttributes<any>>
polyline: SvgStyledFunction<SVGPolylineElement, SVGAttributes<any>>
radialGradient: SvgStyledFunction<SVGRadialGradientElement, SVGAttributes<any>>
rect: SvgStyledFunction<SVGRectElement, SVGAttributes<any>>
stop: SvgStyledFunction<SVGStopElement, SVGAttributes<any>>
svg: SvgStyledFunction<SVGSVGElement, SVGAttributes<any>>
text: SvgStyledFunction<SVGTextElement, SVGAttributes<any>>
tspan: SvgStyledFunction<SVGTSpanElement, SVGAttributes<any>>
export interface SVGComponentFactory {
circle: SvgStyledFunction<SVGCircleElement, SVGProperties>
clipPath: SvgStyledFunction<SVGClipPathElement, SVGProperties>
defs: SvgStyledFunction<SVGDefsElement, SVGProperties>
ellipse: SvgStyledFunction<SVGEllipseElement, SVGProperties>
g: SvgStyledFunction<SVGGElement, SVGProperties>
image: SvgStyledFunction<SVGImageElement, SVGProperties>
line: SvgStyledFunction<SVGLineElement, SVGProperties>
linearGradient: SvgStyledFunction<SVGLinearGradientElement, SVGProperties>
mask: SvgStyledFunction<SVGMaskElement, SVGProperties>
path: SvgStyledFunction<SVGPathElement, SVGProperties>
pattern: SvgStyledFunction<SVGPatternElement, SVGProperties>
polygon: SvgStyledFunction<SVGPolygonElement, SVGProperties>
polyline: SvgStyledFunction<SVGPolylineElement, SVGProperties>
radialGradient: SvgStyledFunction<SVGRadialGradientElement, SVGProperties>
rect: SvgStyledFunction<SVGRectElement, SVGProperties>
stop: SvgStyledFunction<SVGStopElement, SVGProperties>
svg: SvgStyledFunction<SVGSVGElement, SVGProperties>
text: SvgStyledFunction<SVGTextElement, SVGProperties>
tspan: SvgStyledFunction<SVGTSpanElement, SVGProperties>
}
Loading

0 comments on commit f949dde

Please sign in to comment.