0.5.0-alpha.1 - Property 'sx' does not exist on type 'IntrinsicAttributes & ...

[atanasster]

Since upgrading 0.5.0-alpha.1, I see this error on external(non-theme-ui) components

Property 'sx' does not exist on type 'IntrinsicAttributes & ...

example:

https://github.com/ccontrols/component-controls/blob/0335fb94440b8dc67a4b00c7db0560a2f42947aa/ui/components/src/CopyContainer/CopyContainer.tsx#L30

[hasparus]

Okay, so I have one case of this bug, and I think I have a suspect. I didn't check if it's the same thing that happens in component-controls, and don't have a solution yet, but let me share my thoughts.

my case and the investigation

import { jsx, Styled as s } from "theme-ui";

export type BoxedTextProps = ComponentPropsWithoutRef<typeof s.p>;

My BoxedText components spreads all of its props onto Styled.p, so the type of its props is just the type of props of Styled.p.

Simple, right?

Well, no. I must have wrote it before Styled.p or, since we're speaking of types, ThemedComponent<'p'> (defined below) accepted as prop.

export interface ThemedComponent<Name extends ElementType> {
    <As extends ElementType | undefined = undefined>(props: WithPoorAsProp<ComponentProps<Name>, As>): JSX.Element;
}

export declare type WithPoorAsProp<Props, As extends ElementType | undefined = undefined> = {
    as?: As;
} & (As extends undefined ? Props : AnyComponentProps);

interface AnyComponentProps extends JSX.IntrinsicAttributes {
    [key: string]: unknown;
}

WithPoorAsProp is a way to opt out of typechecking a component usage when you add as={"something"}.

But the problem with it is that when we read the props on type level, as becomes ElementType | undefined, and we get a bunch of props mapped to undefined.

Okay, but it should allow any prop, right?

declare type WithConditionalSxProps<P> =
    'className' extends keyof P
        ? P extends { className?: string; }
            ? Omit<P, 'sx'> & { sx: SxProps }
            : Omit<P, keyof 'sx'>
        : Omit<P, 'sx'>;

export declare namespace ThemeUIJSX {
    type LibraryManagedAttributes<C, P> = WithConditionalSxProps<P> & ReactJSXLibraryManagedAttributes<C, P>;
}

WithConditionalSxProps adds sx to our props only if there is a className: string already there.
But wait...

className: unknown should be okay, right? The component want's whatever, and we give it a string.

This is one example of such component.

const PropsToJson = (props: Record<string, unknown>) => <pre>{JSON.stringify(props, null, 2)}</pre> 

We do transform our sx into css on it at runtime, and Emotion transforms css to className.

{ className: string | string[] } is should also receive sx prop, and right now it doesn't.

Let's take a look at WithConditionalSxProps again.

declare type WithConditionalSxProps<P> =
    'className' extends keyof P
        ? P extends { className?: string; }
            ? Omit<P, 'sx'> & { sx: SxProps }
            : Omit<P, keyof 'sx'>
        : Omit<P, 'sx'>;

We need to change P extends { className?: string; } to { className: string; } extends P .

We can read extends as is assignable to or is subset of.
We need to add sx prop to all props object that can accept className prop of type string, so

• if { className: string } is assignable to P
• or if we think of sets, if a set of all objects that have className property of type string is a subset of set P.

Take a look at this TS Playground.. I'll rewrite it to tests in our repo.

---

This leads me to another observation. We could also fix that problem of mapping props to undefined with AnyComponentProps.
What we want to say, in English, is _if as is not defined, the props are the default props of the component, if it's defined anything goes. This isn't what's currently written in TypeScript.

---

Digression

Even if we "support" as prop on components from Styled (this would need few more changes to be done properly), replaced
AnyComponentProps with React.ComponentPropsWithRef<Exclude<As, undefined>> and wrote as={"form" as const} whenever we use it (bcs as="string" doesn't typecheck), we get this

[hasparus]

Okay, I pulled component-controls and checked what's happening on your side @atanasster. Neither TooltipTriggerProps nor PopoverOwnProps declare className?: string, so sx is not added (it's not global anymore).

export type PopoverProps = PopoverOwnProps &
  Omit<Partial<TooltipTriggerProps>, 'children'>;

The type of props you define actually doesn't match what you do at runtime.

export const Popover: FC<PopoverProps> = ({
  arrowVisible = true,
  trigger,
  placement = 'bottom',
  modifiers,
  tooltip,
  children,
  tooltipShown,
  onVisibilityChange,
  ...props
}) => {

You extract arrowVisible, trigger, placement, modifiers, tooltip, children, tooltipShown and onVisiblityChange. You leave the rest as ...props, which you spread onto Box later.

We could define the type as so

interface PopoverProps extends
  PopoverOwnProps,
  Pick<
    TooltipTriggerProps,
    'trigger' | 'placement' | 'modifiers' | 'tooltip' | 'tooltipShown' | 'onVisibilityChange' 
  >,
  BoxProps
{
  // you could specifiy own props here if you don't need to export them separately
}

[atanasster]

Great - I am in the middle of a large coding and will try it asap = possibly tomorrow.

Any specific reason sx is not global anymore?

[hasparus]

Previously it was global even in files without JSX pragma. That was quite error-prone. Right now sx is added only when jsx pragma is used and className can be string. If you opt-in into global jsx pragma with a Babel plugin you also need to opt-in to global types, but we're not polluting the global namespace without asking.

[atanasster]

ahhh thanks, makes sense now

[hasparus]

import {} from 'react'
import { ThemeUIStyleObject } from 'theme-ui'

declare module 'react' {
  interface Attributes {
    sx?: ThemeUIStyleObject
  }
}

I'm wondering if we should add a snippet above in one easy to import file to allow users to opt-in to global sx prop with a one-liner.
@atanasster @dcastil, what do you think?

[atanasster]

@hasparus I personally would love it

[dcastil]

I also think it would be a good idea to expose an importable TS file as an escape hatch. Emotion did the same, I think their code is even the same as in your example.

[atanasster]

just a note, I was unable to quickly move to the new sx props as more issues came about - before this change even if a component did not declare a sx prop, it was automagically applied.

For now, I am re-declaring the global sx and was able to upgrade and give more testing to 0.5.0 in the coming days. I will try to also track down one by one the issues to eventually remove the global sx.