feat(@theme-ui/mdx): add TypeScript support

[LA1CH3]

Converting the @theme-ui/mdx package to TypeScript as mentioned in #668

Couple items of note:

• Updated IntrinsicSxElements interface in @theme-ui/core to use del instead of delete (there exists no delete tag in HTML nor is it an aliased tag)
• Extended tsconfig.json from @theme-ui/core
• Moved @theme-ui/core and @theme-ui/css to devDependencies (leaning on discussion from Adopt TS in theme-ui/color #672 (comment))
• Was a little unsure about the Interpolation type for themed so if there's any help/suggestions for that, would love any comments.

[hasparus]

I think this is a MDX thing as is inlineCode and thematicBreak.
https://github.com/gatsbyjs/gatsby/blob/master/docs/docs/mdx/customizing-components.md

[LA1CH3]

I guess I'm just a bit confused- in v0.3 (which fixed the issue reported in #401), delete was renamed to del.

[hasparus]

I didn’t know about #401. I’m as confused as you are or more. Sorry 😄

[mxstbr]

This looks great, for sure good enough for a first version! 👍 Would you mind also converting the tests in mdx/test/index.js to TypeScript? 🙏

[LA1CH3]

@mxstbr For sure; will have that up later today.

[LA1CH3]

So a couple weird things came up during refactoring of the test.

1. ThemeProvider types from @theme-ui/core enforce that each key of styles supplied to the theme prop must be one of the Sx element types (h1, root, div, etc.). The test in mdx uses a Beep key to style a React component called Beep passed in via MdxProvider. Should we alter the types of ThemeProvider to accept any string keys on styles?

theme-ui/packages/mdx/test/index.js

Line 27 in e3a24ce

Beep: {

theme-ui/packages/css/src/types.ts

Line 581 in e3a24ce

styles?: {

2. It seems like @emotion/styled types are messed up - its unable to recognize the as prop?

<Styled.h1 as={Beep} />

// Prints
Type '{ children: string; as: (props: any) => Element; }' is not assignable to type 'IntrinsicAttributes & ClassAttributes<HTMLHeadingElement> & HTMLAttributes<HTMLHeadingElement> & ThemedProps & { ...; } & { ...; }'.
  Property 'as' does not exist on type 'IntrinsicAttributes & ClassAttributes<HTMLHeadingElement> & HTMLAttributes<HTMLHeadingElement> & ThemedProps & { ...; } & { ...; }'.

Looks like this isn't supported in TS:

emotion-js/emotion#1137

Should we just // @ts-ignore those lines?

[hasparus]

It seems like @emotion/styled types are messed up - its unable to recognize the as prop?
Looks like this isn't supported in TS:

This is hard on TypeScript, but possible.

react-hook-form Controller component has a pretty nice as prop implementation
https://github.com/react-hook-form/react-hook-form/blob/master/src/types.ts#L266-L297

Reakit has correctly typed as prop.

• https://github.com/reakit/reakit/blob/master/packages/reakit-utils/src/types.ts#L82-L92
• https://reakit.io/docs/composition/#as-prop

Andrew Branch from TypeScript team has a nice blogpost on similar approache, which explains why it's heavy on TS and impossible in old language versions:
https://blog.andrewbran.ch/polymorphic-react-components/

[hasparus]

Should we just // @ts-ignore those lines?

It's just my opinion, I'm not part of the team or anything, but
this while this probably is suitable for tests, it makes adopting TypeScript in theme-ui a breaking change for TypeScript and // @ts-check users, because they'd have to ts-ignore it too (actually assertion is a better way).

A PR to emotion to fix the root cause is a big undertaking.
(I'd like to give it a shot one day to continue working on @theme-ui/components.)
A non-breaking solution could allow "opting out of typechecking props" whenever someone uses as prop.

import React, { ElementType } from 'react';

type ThingyOwnProps = { a: number };
type ThingyProps<As extends ElementType | undefined = undefined> =
    { as: As } & (As extends undefined ? ThingyOwnProps : { [key: string]: unknown });

const Thingy = <As extends ElementType | undefined = undefined>(props: ThingyProps<As>) => {
    return <div>{JSON.stringify(props, null, 2)}</div>
}

// Type 'string' is not assignable to type 'number'.(2322) <- Good.
const _1 = <Thingy a="hello" />

// No such component or intrinsic element as "fork". Good.
const _2 = <Thingy as="fork" a="hello" />

// Passing a component works and we opt out of typechecking due to "as" prop.
const SimpleComponent = () => <div />;
const _3 = <Thingy as={SimpleComponent} a="hello" />

// Passing a instrinsic element name works and we also opt out of checking.
const _4 = <Thingy as="form" a="hello" />

TS Playground

[jxnblk]

While I think it's best to avoid breaking changes as much as possible, we are planning on bumping to 0.4 for the TS conversion, since there might be some unforeseen breaking changes anyway.

As far as the styles.Beep issue goes, I think it's worth considering whether we'd want to lock down the theme.styles object to be more strict (limited to MDX elements) or whether it should be as extensible as any other variant object (theme.styles is a variant for MDX documents/components)

[LA1CH3]

I like that approach @hasparus, it does suck a bit to lose typed props when the as prop is present but that seems like it may be the best solution in the interim. Thanks for putting together the TS playground example- made it very clear. Thoughts on going forward with this?

On locking down theme.styles- in the context of the MDX package, the documentation demonstrates its usage for styling MDX elements (an h1 in this case), but since its currently capable of styling components as well, that would be a breaking change if it were to be made limited to MDX elements. For the types to reflect current functionality, the theme.styles definition would have to be altered to assume any other named keys besides MDX/theme-ui element keys are React components.

On whether it should be kept this way going forward or made to be stricter, that may warrant seeing how its used in the community and seeing if people are theming React components through styles or not. I lean towards allowing it to be more open.

[hasparus]

@LA1CH3

I like that approach @hasparus, it does suck a bit to lose typed props when the as prop is present but that seems like it may be the best solution in the interim. Thanks for putting together the TS playground example- made it very clear. Thoughts on going forward with this?

This blocks TS adoption in @theme-ui/components (Paulie's #692 and my PaulieScanlon#1), and I'd really like to remove @types/theme-ui__components from DT, because it will be bad for my email inbox in the long term :D

Loose idea:

How to fix the as prop problem in @emotion/styled-base

1. Fork emotion
   • TypeScript typedefs for @emotion/styled-base are in emotion/packages/styled-base/types/index.d.ts. Last commit to the file was 8 months ago :D
   • Change required TS version to 3.5
   • The logic for as prop is in emotion/packages/styled-base/src/index.js#L86
2. Hack a proof of concept
3. Contact Andarist or someone else from Emotion team to speak about it. This would be polite, because we require higher TypeScript version. I have a bad feeling that all changes to emotion styled typings may be breaking for TS users (like all my apps at work ouch).
4. @emotion/styled-base is currently tested with dtslint. I am not sure if it's sufficient. Do we need tsd or even ts-snippet? We'd like to assert that the props are inferred properly, because we kinda want to test "developer experience".
5. Create a PR for Emotion

[hasparus]

I think this can be closed, as we merged #1031 with LA1CH3's commit.