plotly / dash-typescript-component-template

Create Dash components using TypeScript
19 stars 4 forks source link

Possible to pick from `React.HTMLAttributes`? #4

Closed frnhr closed 2 years ago

frnhr commented 2 years ago

When making a Dash component based on existing React TS component, it would be useful to be able to extend the component's interface instead of specifying each prop anew.

I believe this should work in principle but in practice when I try to inherit from the underlying component's interface, the result is an empty object.

Consider this dummy component:

import React from 'react'
import { DashComponentProps } from '../props'

type Props = {
    /**
     * Variant is the name of a variation of the button so that buttons can vary very variably.
     */
    variant: 'yellow' | 'green';
}
    & DashComponentProps
    & React.ButtonHTMLAttributes<HTMLButtonElement>  // <--- this is the problematic line

const Button = (props: Props) => {
    props.variant = "green"
    props.title = "foo bar"
    return <button {...props}>{props.children}</button>
}

Button.defaultProps = {}
export default Button

Building (yarn build) produces a Button class in Button.py with zero attributes. šŸ˜±

I have traced the issue to props = getPropInfo(propsType, defaultProps) line in extract-meta.js, but I'm kind of out of my depth WRT Typescript here.

My plan was to pick a few props from the original interface. For example, autofocus:

    & DashComponentProps
    & Pick<React.ButtonHTMLAttributes<HTMLButtonElement>, 'autoFocus'>

I was expecting this to add a autoFocus?: boolean | undefined; to the Prop type, but what I actually get from getPropInfo in extract-meta.js at that point is:

Notification_Center

I would expect something like "type": {"name": "bool", "raw": "boolean"} instead of "type":{"name": "shape", "value": Object, "raw": "ButtonHTMLAttributes<HTMLButtonElement>"}

Or am I doing something terribly wrong here?

Jordan-Hall commented 2 years ago

Hey @frnhr The code you produce is valid and should work

I have traced the issue to props = getPropInfo(propsType, defaultProps) line in extract-meta.js, but I'm kind of out of my depth WRT Typescript here.

Where is this method, I'll happy take a look at this as I'll need this to work on my own project.

frnhr commented 2 years ago

Thank you for your interest! This is the line in question:

https://github.com/plotly/dash/blob/cebad0734215cc12a300a0173859cf1b4ea43bb0/dash/extract-meta.js#L689 (looking in dev branch)

I don't think there is anything wrong with it, exactly. It's just that my debugging went that far. It seems that inside getPropInfo the metadata is being extracted by using ts.createProgram and friends, and that's where my respiration rate starts climbing too high for comfort šŸ˜„

Jordan-Hall commented 2 years ago

Thank you, I'll take a look in that now, I'm use to ts.createProgram now so hopefully it wont take too long to find the issue.

Jordan-Hall commented 2 years ago

@frnhr So it looks like typescript has an issue with namepaces for the type.

import React, { ButtonHTMLAttributes } from 'react' will resolve the issue for you.

Still looking in to double check my assumption but I think this can be closed now. cc: @T4rk1n

T4rk1n commented 2 years ago

Namespaces has issues, most notably local namespace are not supported but React or JSX should work fine as long as React is imported. I think the React Html attributes doesn't work because there are props that are not supported by the compiler.

I have these as invalids for wrapping react html attributes:

type InvalidHtmlProps =
    | 'dangerouslySetInnerHTML'
    | 'defaultChecked'
    | 'defaultValue'
    | 'suppressContentEditableWarning'
    | 'suppressHydrationWarning'
    | 'ref'
    | 'key'
    | 'async';

type EventsProps = keyof Omit<React.DOMAttributes<any>, 'children'>;
type AriaProps = keyof React.AriaAttributes;
export type HtmlOmittedProps = InvalidHtmlProps | EventsProps | AriaProps;
T4rk1n commented 2 years ago

import React, { ButtonHTMLAttributes } from 'react' will resolve the issue for you.

@Jordan-Hall I tried that and it pulls the right props, so it's indeed a limitation with the namespace, thanks for the investigation.

I think in the compiler it should pull the parent or something to that effect when it encounter a namespace.

Jordan-Hall commented 2 years ago

I believe the ts.program does this already. I've reached out to some people I know who really good at TS compiler. I've done alot with custom compliers but this is confusing me slightly.

T4rk1n commented 2 years ago

@Jordan-Hall Don't worry I got the fix coming up.

Running ts.getPreEmitDiagnostics(program), the tsconfig of the program did not get the right config value, it needs esModuleInterop to be true and jsx to 'react'.

Jordan-Hall commented 2 years ago

Perfect. Thank you! Great work on this, the currently way with porotypes just not suitable for the project I'm working on. So been looking at this one to solve our issues

frnhr commented 2 years ago

The fix from PR https://github.com/plotly/dash/pull/2053 seems to fix my issue in full.

Thank you both for the quick resolution! It's great to see open source at its best! šŸš€