Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid printing out expected/actual types in JSX attributes unless a spread is provided #35253

Open
DanielRosenwasser opened this issue Nov 20, 2019 · 2 comments
Labels
Domain: Error Messages The issue relates to error messaging Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". Experience Enhancement Noncontroversial enhancements Help Wanted You can do this
Milestone

Comments

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Nov 20, 2019

Playground link

import React, { HTMLAttributes, ReactNode, } from 'react';


type EuiIcon = React.PureComponent;

<Badge type="#fff">
  sup
</Badge>

function Badge(props: BadgeProps) {
  return <div />
}

export type BadgeProps = {
  /**
   * Accepts any string from our icon library
   */
  iconType?: string | React.ReactElement;

  /**
   * The side of the badge the icon should sit
   */
  iconSide?: IconSide;

  /**
   * Accepts either our palette colors (primary, secondary ..etc) or a hex value `#FFFFFF`, `#000`.
   */
  color?: IconColor;
  /**
   * Will override any color passed through the `color` prop.
   */
  isDisabled?: boolean;

  /**
   * Props passed to the close button.
   */
  closeButtonProps?: Partial<PropsOf<EuiIcon>>;
} & CommonProps &
  ExclusiveUnion<WithIconOnClick, {}> &
  ExclusiveUnion<WithSpanProps, WithButtonProps>;

type IconColor = string;

interface WithIconOnClick {
  /**
   * Will apply an onclick to icon within the badge
   */
  iconOnClick: MouseEventHandler<HTMLButtonElement>;

  /**
   * Aria label applied to the iconOnClick button
   */
  iconOnClickAriaLabel: string;
}

type WithSpanProps = Omit<HTMLAttributes<HTMLSpanElement>, 'onClick' | 'color'>;

type IconSide = 'left' | 'right';

import {
  AnchorHTMLAttributes,
  ButtonHTMLAttributes,
  Component,
  FunctionComponent,
  MouseEventHandler,
  SFC,
} from 'react';

export interface CommonProps {
  className?: string;
  'aria-label'?: string;
  'data-test-subj'?: string;
}

export type PropsOf<C> = C extends SFC<infer SFCProps>
  ? SFCProps
  : C extends FunctionComponent<infer FunctionProps>
  ? FunctionProps
  : C extends Component<infer ComponentProps>
  ? ComponentProps
  : never;

type UnionKeys<T> = T extends any ? keyof T : never;
export type DistributivePick<T, K extends UnionKeys<T>> = T extends any
  ? Pick<T, Extract<keyof T, K>>
  : never;
export type DistributiveOmit<T, K extends UnionKeys<T>> = T extends any
  ? Omit<T, Extract<keyof T, K>>
  : never;

/**
 * Returns member keys in U not present in T set to never
 * T = { 'one', 'two', 'three' }
 * U = { 'three', 'four', 'five' }
 * returns { 'four': never, 'five': never }
 */
export type DisambiguateSet<T, U> = {
  [P in Exclude<keyof T, keyof U>]?: never
};

/**
 * Allow either T or U, preventing any additional keys of the other type from being present
 */
export type ExclusiveUnion<T, U> = (T | U) extends object // if there are any shared keys between T and U
  ? (DisambiguateSet<T, U> & U) | (DisambiguateSet<U, T> & T) // otherwise the TS union is already unique
  : T | U;

type WithButtonProps = {
  /**
   * Will apply an onclick to the badge itself
   */
  onClick: MouseEventHandler<HTMLButtonElement>;

  /**
   * Aria label applied to the iconOnClick button
   */
  onClickAriaLabel: string;
} & Omit<HTMLAttributes<HTMLButtonElement>, 'onClick' | 'color'>;

Current

Type '{ children: string; type: string; }' is not assignable to type '(IntrinsicAttributes & { iconType?: string | ReactElement<any, string | ((props: any) => ReactElement<any, string | ... | (new (props: any) => Component<any, any, any>)> | null) | (new (props: any) => Component<...>)> | undefined; iconSide?: "left" | ... 1 more ... | undefined; color?: string | undefined; isDisabled...'.
  Property 'type' does not exist on type '(IntrinsicAttributes & { iconType?: string | ReactElement<any, string | ((props: any) => ReactElement<any, string | ... | (new (props: any) => Component<any, any, any>)> | null) | (new (props: any) => Component<...>)> | undefined; iconSide?: "left" | ... 1 more ... | undefined; color?: string | undefined; isDisabled...'.(2322)

Proposed

The 'Badge' tag does not accept any attribute named 'type'.
@DanielRosenwasser DanielRosenwasser added Domain: Error Messages The issue relates to error messaging Experience Enhancement Noncontroversial enhancements labels Nov 20, 2019
@DanielRosenwasser DanielRosenwasser added Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". Help Wanted You can do this labels Jun 1, 2020
@DanielRosenwasser DanielRosenwasser added this to the Backlog milestone Jun 1, 2020
@DanielRosenwasser
Copy link
Member Author

There's a lot of subtleties - any implementation should account for

  • overloads where a property is declared in one but not the other
  • generic signatures of some sort

under these cases, I think that a false positive (i.e. reporting this new error when it's not appropriate) is not acceptable because it could be so much more confusing.

@tbtommyb
Copy link

I would like to take this on, since these errors are a bugbear of mine.

I will look into the code and can I revert to you here / on Discord if I have any queries? thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Domain: Error Messages The issue relates to error messaging Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". Experience Enhancement Noncontroversial enhancements Help Wanted You can do this
Projects
None yet
Development

No branches or pull requests

2 participants