Skip to content

Commit

Permalink
fix(types): don't lock csstype version
Browse files Browse the repository at this point in the history
  • Loading branch information
hasparus committed Oct 24, 2024
1 parent 28d1a87 commit 401b794
Show file tree
Hide file tree
Showing 6 changed files with 15 additions and 123 deletions.
2 changes: 1 addition & 1 deletion packages/css/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
"access": "public"
},
"dependencies": {
"csstype": "3.0.10"
"csstype": "^3.0.10"
},
"peerDependencies": {
"@emotion/react": "^11.11.1"
Expand Down
63 changes: 0 additions & 63 deletions packages/css/test/errors-and-inference.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,42 +7,6 @@ const expectSnippet = expecter(`
`)

describe('Theme', () => {
test('shows friendly error only on bad property', () => {
expectSnippet(`
css({
bg: 'salmon',
whiteSpace: 'no-works',
'> form': {
color: 'blue',
widows: 'bar',
// unknown CSS property is accepted
whitePace: 'this-works',
},
})
`).toFail(
/Error snippet\.tsx \(\d+,\d+\): Type '"no-works"' is not assignable to type [\s\S]+'./
)
})

test('shows friendly error on nested object', () => {
expectSnippet(`
css({
bg: 'salmon',
'> form': {
color: 'blue',
whiteSpace: 'banana',
},
})
`).toFail(
new RegExp(
`Error snippet\\.tsx \\(\\d+,\\d+\\): Type '{ color: "blue"; whiteSpace: "banana"; }'` +
` is not assignable to type '[\\s\\S]+'.\\n\\s+` +
`Types of property 'whiteSpace' are incompatible.\\n\\s+` +
`Type '"banana"' is not assignable to type [\\s\\S]+`
)
)
})

test('accepts unknown CSS property without error', () => {
expect(css({ '> form': { windows: 'baz' } })({})).toStrictEqual({
'> form': { windows: 'baz' },
Expand Down Expand Up @@ -96,9 +60,6 @@ describe('Theme', () => {
css({ size: (t) => get(t, 'space.3') + get(t, 'sizes.5') })

const parse = (x: string | number | undefined | {}) => parseInt(String(x))
css({
size: (t) => parse(t.space?.[3]) + parse(t.sizes?.[5]),
})

// Current limitation. If you broke this one, that's actually pretty awesome,
// but TypeScript chapter in the docs needs an update.
Expand All @@ -110,30 +71,6 @@ describe('Theme', () => {
})
})

// This is not a feature, but the TypeScript chapter in the docs will need an
// update if this test fails.
test('inferred type `string` is too wide for `whiteSpace`', () => {
expectSnippet(`
const style = {
whiteSpace: 'pre-line'
}
css(style);
`).toFail(
/Type '{ whiteSpace: string; }' is not assignable to type 'ThemeUICSSObject'./
)

expectSnippet(`
import { ThemeUICSSObject } from './packages/css'
const style: ThemeUICSSObject = {
whiteSpace: 'pre-line'
}
css(style);
`).toSucceed()
})

describe('ColorMode', () => {
const expectedSnippet = expectSnippet(`
import { ColorMode } from './packages/css/src'
Expand Down
7 changes: 7 additions & 0 deletions packages/css/test/past-bugs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,10 @@ describe('theme scales, get and default object property (#1439)', () => {
expect(actual).toStrictEqual({ zIndex: 1 })
})
})

// https://github.com/system-ui/theme-ui/issues/2520
it('accepts number as aspect ratio', () => {
const actual = css({ aspectRatio: 0.5 })({})

expect(actual).toStrictEqual({ aspectRatio: 0.5 })
})
47 changes: 0 additions & 47 deletions packages/docs/src/pages/guides/typescript.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -127,50 +127,3 @@ const syntaxHighlighting = theme.syntaxHighlighting

_[Try it in TypeScript Playground.](https://www.typescriptlang.org/v2/en/play?#code/JYWwDg9gTgLgBAbzgFQBYFMTrgXzgMyghDgHIYMsBaAV2FIG4AoJ4AOxnSnwEMBjbAFkAngGVhHHgA8AEsADmqADYLUMdvLSZsCJnALR08ojTYATAFxwAzjCgbmOFmfR8lPKNhAQzNJdnJKdFp6RD04dk5ufmwtLDD9fWsJGGk5RRVFdTZ5KxFxSVlVTLUNOPRwpycmPgg2WzgKbStyuABeBJsUtOLVbNzO-XxDYwhTSzIAYgAmWdIAGkqmHGYauobkwvTlPo12xqCAOk3UoozdnLX6+C4iKH2mrGPhGDYe86yNJiA)_

## Common Problems

### Union types are not inferred without explicit annotation

Style objects defined outside of `css` function and `sx` prop need explicit
annotation to prevent following error.

```tsx
/** @jsxImportSource theme-ui */

const style = { whiteSpace: 'pre-line' }

// Type '{ whiteSpace: string; }' is not assignable to type 'ThemeUICSSObject'.
// Type 'string' is not assignable to type '"inherit" | "initial" | "revert" | "unset" | "normal" | "break-spaces"
return <div sx={style} />
```

_[Try it on CodeSandbox.](https://codesandbox.io/s/theme-ui-inferrence-too-wide-vkrf5?file=/src/index.tsx&view=editor&previewwindow=tests)_

TypeScript assumes that `whiteSpace` here is a `string`, but the `whiteSpace`
property in `ThemeUICSSObject` is a union of possible white-space values
([see on MDN](https://developer.mozilla.org/en-US/docs/Web/CSS/white-space#Values))
or a nested style object.

We can explicitly annotate `style` ensure that it is a correct Theme UI style
object and that `whiteSpace` is one of appropriate values.

```tsx
/** @jsxImportSource theme-ui */
import { ThemeUICSSObject } from 'theme-ui'

const style: ThemeUICSSObject = { whiteSpace: 'pre-line' }

// No error
return <div sx={style} />
```

_[Try it on CodeSandbox.](https://codesandbox.io/s/theme-ui-inferrence-too-wide-vkrf5?file=/src/index.tsx&view=editor&previewwindow=tests)_

We could also fix our problem by narrowing the type of `style` with a
[const assertion](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-4.html#const-assertions).

```tsx
const style = { whiteSpace: 'pre-line' } as const
```

This is succinct, but error prone, because we won't get TS intellisense support inside of this object.
2 changes: 1 addition & 1 deletion packages/typography/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
"@types/modularscale": "^2.0.0",
"@types/typography": "^0.16.4",
"compass-vertical-rhythm": "^1.4.5",
"csstype": "3.0.10",
"csstype": "^3.0.10",
"modularscale": "^2.0.1",
"type-fest": "^2.18.0"
},
Expand Down
17 changes: 6 additions & 11 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 401b794

Please sign in to comment.