Merge pull request #1308 from system-ui/fix-jsx-types

fix(types): add sx prop whenever className can be string

.github/workflows/ci.yml

@@ -1,27 +1,34 @@
 name: CI
 
-on: [push, pull_request]
+on:
+  push:
+  pull_request:
+    branches:
+      # Branches from forks have the form 'user:branch-name' so we only run
+      # this job on pull_request events for branches that look like fork
+      # branches. Without this we would end up running this job twice for non
+      # forked PRs, once for the push and then once for opening the PR.
+      - '**:**'
 
 jobs:
   build:
-
     runs-on: ubuntu-latest
 
     steps:
-    - uses: actions/checkout@v2
-    - uses: actions/setup-node@v1
-      with: 
-        node-version: 12.x
-    - name: Get yarn cache directory path
-      id: yarn-cache-dir 
-      run: echo "::set-output name=dir::$(yarn cache dir)"
+      - uses: actions/checkout@v2
+      - uses: actions/setup-node@v1
+        with:
+          node-version: 12.x
+      - name: Get yarn cache directory path
+        id: yarn-cache-dir
+        run: echo "::set-output name=dir::$(yarn cache dir)"
 
-    - uses: actions/cache@v1
-      with:
-        path: ${{ steps.yarn-cache-dir.outputs.dir }}
-        key: ${{ runner.os }}-yarn-${{ hashFiles('**/yarn.lock') }}
-        restore-keys: |
-          ${{ runner.os }}-yarn-
-    - run: yarn
-    - run: yarn test --coverage
-    - run: yarn typecheck
+      - uses: actions/cache@v1
+        with:
+          path: ${{ steps.yarn-cache-dir.outputs.dir }}
+          key: ${{ runner.os }}-yarn-${{ hashFiles('**/yarn.lock') }}
+          restore-keys: |
+            ${{ runner.os }}-yarn-
+      - run: yarn
+      - run: yarn test --coverage
+      - run: yarn typecheck

CHANGELOG.md

@@ -2,6 +2,12 @@
 
 ## v0.5.0 UNRELEASED
 
+- **breaking TypeScript**: Renamed and removed types. PR #1308
+  - `SxProps` to `SxProp`.
+  - `SxStyleProp`, an alias for `ThemeUIStyleObject` removed. Use `ThemeUIStyleObject` instead.
+- Fix: Add `sx` props types to all props accepting `className`. 
+- Fix WithPoorAsProp to work with ComponentProps utility type.
+
 ## v0.5.0-alpha.2 2020-11-30
 
 - Add Paragraph component. PR #1298
@@ -10,6 +16,7 @@
 
 - Bump React peerDependency to `"^16.14.0 || ^17.0.0"`.
 - Support automatic JSX runtime. Issue #1160, PR #1237
+- Bump React peerDependency to `"^16.14.0 || ^17.0.0"`.
 - Apply variant styles _before_ responsive styles. Issues #1030, and #720, PR #1273
 
 ## v0.5.0-alpha.0 2020-11-20

package.json

@@ -47,7 +47,6 @@
     "prettier": "^2.2.0",
     "react-test-renderer": "^16.8.6",
     "ts-jest": "^26.4.4",
-    "ts-snippet": "^5.0.0",
     "typescript": "^4"
   },
   "jest": {

packages/core/src/index.ts

@@ -1,4 +1,7 @@
-import { jsx as emotion, ThemeContext as EmotionContext } from '@emotion/react'
+import {
+  jsx as emotionJsx,
+  ThemeContext as EmotionContext,
+} from '@emotion/react'
 import { Theme } from '@theme-ui/css'
 import * as React from 'react'
 import deepmerge from 'deepmerge'
@@ -33,8 +36,11 @@ export * from './types'
 
 const __EMOTION_VERSION__ = packageInfo.version
 
-export const jsx: typeof React.createElement = (type, props, ...children) =>
-  emotion.apply(undefined, [type, parseProps(props), ...children])
+export const jsx: typeof React.createElement = <P extends {}>(
+  type: React.FunctionComponent<P> | React.ComponentClass<P> | string,
+  props: React.Attributes & P,
+  ...children: React.ReactNode[]
+): any => emotionJsx(type, parseProps(props), ...children)
 
 export declare namespace jsx {
   export namespace JSX {
@@ -73,28 +79,29 @@ const canUseSymbol = typeof Symbol === 'function' && Symbol.for
 const REACT_ELEMENT = canUseSymbol ? Symbol.for('react.element') : 0xeac7
 const FORWARD_REF = canUseSymbol ? Symbol.for('react.forward_ref') : 0xeac7
 
-const isMergeableObject = (n) => {
-  return (
-    !!n &&
-    typeof n === 'object' &&
-    n.$$typeof !== REACT_ELEMENT &&
-    n.$$typeof !== FORWARD_REF
-  )
+const deepmergeOptions: deepmerge.Options = {
+  isMergeableObject: (n) => {
+    return (
+      !!n &&
+      typeof n === 'object' &&
+      (n as React.ExoticComponent).$$typeof !== REACT_ELEMENT &&
+      (n as React.ExoticComponent).$$typeof !== FORWARD_REF
+    )
+  },
+  arrayMerge: (_leftArray, rightArray) => rightArray,
 }
 
-const arrayMerge = (destinationArray, sourceArray, options) => sourceArray
-
 /**
  * Deeply merge themes
  */
 export const merge = (a: Theme, b: Theme): Theme =>
-  deepmerge(a, b, { isMergeableObject, arrayMerge })
+  deepmerge(a, b, deepmergeOptions)
 
 function mergeAll<A, B>(a: A, B: B): A & B
 function mergeAll<A, B, C>(a: A, B: B, c: C): A & B & C
 function mergeAll<A, B, C, D>(a: A, B: B, c: C, d: D): A & B & C & D
 function mergeAll<T = Theme>(...args: Partial<T>[]) {
-  return deepmerge.all<T>(args, { isMergeableObject, arrayMerge })
+  return deepmerge.all<T>(args, deepmergeOptions)
 }
 
 merge.all = mergeAll

packages/core/src/jsx-namespace.ts

@@ -1,10 +1,10 @@
-import { SxProps } from './types'
+import { SxProp } from './types'
 
-type WithConditionalSxProps<P> = 'className' extends keyof P
-  ? P extends { className?: string }
-    ? Omit<P, keyof SxProps> & SxProps
-    : Omit<P, keyof SxProps>
-  : Omit<P, keyof SxProps>
+type WithConditionalSxProp<P> = 'className' extends keyof P
+  ? string extends P['className']
+    ? P & SxProp
+    : P
+  : P
 
 type ReactJSXElement = JSX.Element
 type ReactJSXElementClass = JSX.ElementClass
@@ -22,13 +22,20 @@ export declare namespace ThemeUIJSX {
     extends ReactJSXElementAttributesProperty {}
   export interface ElementChildrenAttribute
     extends ReactJSXElementChildrenAttribute {}
-  export type LibraryManagedAttributes<C, P> = WithConditionalSxProps<P> &
+  export type LibraryManagedAttributes<C, P> = WithConditionalSxProp<P> &
+    // We are not removing incompatible `sx` props, because touching this breaks
+    // inference in generic components.
+    // Yes, we steal any prop called `sx` at runtime, but we
+    // can't represent it on type level without breaking compatibility with
+    // our own Field, react-hook-form, and a bunch of other generic components.
+    // Don't touch ReactJSXLibraryManagedAttributes or you'll spend hours
+    // debugging and entirely spoil your day.
     ReactJSXLibraryManagedAttributes<C, P>
   export interface IntrinsicAttributes extends ReactJSXIntrinsicAttributes {}
   export interface IntrinsicClassAttributes<T>
     extends ReactJSXIntrinsicClassAttributes<T> {}
   export type IntrinsicElements = {
     [K in keyof ReactJSXIntrinsicElements]: ReactJSXIntrinsicElements[K] &
-      SxProps
+      SxProp
   }
 }

packages/core/src/types.ts

@@ -1,15 +1,8 @@
-import { ThemeUIStyleObject, CSSObject } from '@theme-ui/css'
+import { ThemeUIStyleObject } from '@theme-ui/css'
 
 import { ThemeUIJSX } from './jsx-namespace'
 
-/**
- * The `sx` prop accepts a `SxStyleProp` object and properties that are part of
- * the `Theme` will be transformed to their corresponding values. Other valid
- * CSS properties are also allowed.
- */
-export type SxStyleProp = ThemeUIStyleObject
-
-export interface SxProps {
+export interface SxProp {
   /**
    * The sx prop lets you style elements inline, using values from your
    * theme. To use the sx prop, add the custom pragma as a comment to the
@@ -21,7 +14,7 @@ export interface SxProps {
    * import { jsx } from 'theme-ui'
    * ```
    */
-  sx?: SxStyleProp
+  sx?: ThemeUIStyleObject
 }
 
 export interface IntrinsicSxElements {

packages/core/test/__snapshots__/react-jsx.tsx.snap

@@ -9,7 +9,16 @@ exports[`JSX accepts sx prop 1`] = `
   scroll-padding-bottom: 8px;
 }
 
+.emotion-1 {
+  bg-color: primary;
+}
+
 <div
   className="emotion-0"
-/>
+>
+  <input
+    className="emotion-1"
+    onChange={[Function]}
+  />
+</div>
 `;

packages/core/test/index.tsx

@@ -1,7 +1,5 @@
 import React from 'react'
-import { mdx } from '@mdx-js/react'
-import renderer from 'react-test-renderer'
-import { render, fireEvent, cleanup, act } from '@testing-library/react'
+import { cleanup } from '@testing-library/react'
 import { renderJSON } from '@theme-ui/test-utils'
 import { matchers } from '@emotion/jest'
 import mockConsole from 'jest-mock-console'
@@ -333,7 +331,6 @@ describe('merge', () => {
   })
 })
 
-// describe('Context', () => {})
 describe('useThemeUI', () => {
   test('returns theme context', () => {
     let context: ContextValue | undefined

packages/core/test/react-jsx.tsx

@@ -1,8 +1,7 @@
 /** @jsx jsx */
-import renderer from 'react-test-renderer'
-import { renderJSON } from '@theme-ui/test-utils'
+import { renderJSON, NotHas, Assert, IsExact } from '@theme-ui/test-utils'
 
-import { jsx } from '../src'
+import { jsx, SxProp, ThemeUIJSX } from '../src'
 
 describe('JSX', () => {
   test('accepts sx prop', () => {
@@ -14,9 +13,59 @@ describe('JSX', () => {
             mt: 10,
             px: 2,
             scrollPaddingY: 2,
-          }}
-        />
+          }}>
+          <input
+            onChange={(e) => console.log(e.target.value)}
+            sx={{
+              bgColor: 'primary',
+            }}
+          />
+        </div>
       )
     ).toMatchSnapshot()
   })
 })
+
+{
+  type HasSxProp<T extends SxProp> = T extends SxProp ? true : false
+  type DoesNotHaveSxProp<T extends object> = NotHas<T, 'sx'>
+
+  type _ =
+    | Assert<
+        DoesNotHaveSxProp<
+          ThemeUIJSX.LibraryManagedAttributes<
+            React.FC,
+            { className?: undefined }
+          >
+        >,
+        true
+      >
+    | Assert<
+        | HasSxProp<
+            ThemeUIJSX.LibraryManagedAttributes<
+              React.FC,
+              { className?: string; anotherProp: string; andOneMore: number }
+            >
+          >
+        | HasSxProp<
+            ThemeUIJSX.LibraryManagedAttributes<React.FC, { className: string }>
+          >
+        // if `className` can be whatever, we have `sx` prop
+        | HasSxProp<
+            ThemeUIJSX.LibraryManagedAttributes<
+              React.FC,
+              { className?: unknown }
+            >
+          >
+        // if `className` can be string or many, we have `sx` prop
+        | HasSxProp<
+            ThemeUIJSX.LibraryManagedAttributes<
+              React.FC,
+              {
+                className?: string | string[]
+              }
+            >
+          >,
+        true
+      >
+}

packages/core/tsconfig.json

@@ -2,7 +2,6 @@
   "extends": "../../tsconfig.json",
   "compilerOptions": {
     "jsx": "react",
-    "strict": false,
     "strictFunctionTypes": true
   },
   "include": ["src/**/*.ts", "src/**/*.tsx", "src/**/*.d.ts"]

packages/css/tsconfig.json

@@ -1,7 +1,4 @@
 {
   "extends": "../../tsconfig.json",
-  "compilerOptions": {
-    "strict": true
-  },
   "include": ["src/**/*.ts", "src/**/*.tsx", "src/**/*.d.ts"]
 }

packages/editor/src/Combobox.tsx

@@ -1,5 +1,5 @@
 /** @jsx jsx */
-import { jsx, SxProps } from 'theme-ui'
+import { jsx, SxProp } from 'theme-ui'
 import { useState, useRef, useEffect } from 'react'
 import { Label, Input, InputProps } from '@theme-ui/components'
 
@@ -37,8 +37,8 @@ type ComboboxOwnProps<T> = {
 }
 export interface ComboboxProps<T>
   extends ComboboxOwnProps<T>,
-    Omit<InputProps, keyof ComboboxOwnProps<T> | keyof SxProps>,
-    SxProps {}
+    Omit<InputProps, keyof ComboboxOwnProps<T> | keyof SxProp>,
+    SxProp {}
 
 const Combobox = <T extends string | number>({
   type = 'text',

packages/editor/src/Sx/Space.tsx

@@ -1,5 +1,5 @@
 /** @jsx jsx */
-import { jsx, SxStyleProp } from 'theme-ui'
+import { jsx, ThemeUIStyleObject } from 'theme-ui'
 import { ThemeUIExtendedCSSProperties } from '@theme-ui/css'
 import { useState, useEffect } from 'react'
 import { Field, Label, Checkbox } from '@theme-ui/components'
@@ -36,7 +36,7 @@ export interface SpaceProps {
       | 'my'
     >
   >
-  onChange: (sx: SxStyleProp) => void
+  onChange: (sx: ThemeUIStyleObject) => void
 }
 
 export const Space = ({

packages/mdx/src/index.ts

@@ -99,7 +99,7 @@ export type WithPoorAsProp<
   As extends ElementType | undefined = undefined
 > = {
   as?: As
-} & (As extends undefined ? Props : AnyComponentProps)
+} & (undefined extends As ? Props : AnyComponentProps)
 
 export interface ThemedComponent<Name extends ElementType> {
   <As extends ElementType | undefined = undefined>(

packages/sidenav/tsconfig.json

@@ -1,7 +1,6 @@
 {
   "extends": "../../tsconfig.json",
   "compilerOptions": {
-    "strict": true,
     "jsx": "react"
   },
   "include": ["src/**/*.ts", "src/**/*.tsx", "src/**/*.d.ts"]

packages/test-utils/package.json

@@ -12,5 +12,9 @@
     "prepare": "tsc",
     "watch": "tsc -w",
     "typecheck": "tsc --noEmit"
+  },
+  "dependencies": {
+    "ts-snippet": "^5.0.0",
+    "conditional-type-checks": "^1.0.5"
   }
 }