Typescript: need data-test-subj prop available to set on some eui types (or maybe all?)

[stacey-gammon]

Looks like I can't add data-test-subj without typescript complaining. I'm thinking we need a more general way of allowing this and other html default props to be passed along.

[cchaos]

@stacey-gammon Out of curiosity, does it not work to change it from a string 'data-test-subj' to a camelCase key dataTestSubj? I thought if certain props were not explicitly stated in the component it should be applied to the component's attributes via ...rest but React expects attributes to camelCase and then it changes it to the typical - notation.

Or maybe it's different with TS...

[stacey-gammon]

Oh interesting, I didn't know that. Still unfortunately looks like it doesn't work with typescript:

Object literal may only specify known properties, and 'dataTestSubj' does not exist in type 'EuiContextMenuPanelItemDescriptor'.

[chandlerprall]

TypeScript's jsx docs says

Note: If an attribute name is not a valid JS identifier (like a data-* attribute), it is not considered to be an error if it is not found in the element attributes type.

I'm not sure where that is coded, if it's part of the JSX ts interface or deeper in typescript. I think the first step would be to play with declaring the EUI component as one or more of the JSX interfaces/classes.

The intrinsic JSX elements (those that are specified lowercase) all include React.HTMLAttributes, for example div is defined as React.DetailedHTMLProps<React.HTMLAttributes<HTMLDivElement>, HTMLDivElement>.

[chandlerprall]

Alternatively, we could shift from using ...rest and having an explicit property on components which works as the catch-all. <EuiComponent passthrough={{'data-test-subj': 'id5'}}>. This prop would be typed as {[key: string]: any}

[chandlerprall]

@stacey-gammon looks like, in this instance, the EuiContextMenuItemProps interface is missing CommonProps, can you try making this change in your installed EUI sources to verify:

context_menu/index.d.ts line 58

  export interface EuiContextMenuItemProps {
    icon?: EuiContextMenuItemIcon;
    hasPanel?: boolean;
    buttonRef?: RefCallback<HTMLButtonElement>;
  }

to

  export interface EuiContextMenuItemProps extends CommonProps {
    icon?: EuiContextMenuItemIcon;
    hasPanel?: boolean;
    buttonRef?: RefCallback<HTMLButtonElement>;
  }

EuiContextMenuPanelItemDescriptor is based on EuiContextMenuItemProps and this change should expose data-test-subj to that value.

[stacey-gammon]

Yes that worked @chandlerprall! Now I get an error about disabled not being a valid prop. I think that one might be missing?

[stacey-gammon]

And onClick too. I have this so far:

export interface EuiContextMenuItemProps extends CommonProps {
    icon?: EuiContextMenuItemIcon;
    hasPanel?: boolean;
    disabled?: boolean;
    onClick?: () => void;
    buttonRef?: RefCallback<HTMLButtonElement>;
  }

[chandlerprall]

@stacey-gammon would you mind creating a PR with those changes?

[stacey-gammon]

Yea, I can do that, but I wanted to fix everything I needed in one PR and I am blocked right now on #985

[stacey-gammon]

tl;dr I think Icons should be allowed to be non-react so plugins don't have to be written in react. But the given types, i think force them to be react (ReactElement<any>)

[chandlerprall]

Thanks! I'll be making related changes in another component later today which should influence how to pass HTMLElements around. I'll update when there's some progress :)

[chandlerprall]

Closing this as the original issue was addressed in #1006. There's another issue for allowing HTML elements as children - #1098