Skip to content

Props requiring casting on complex type (and defaultProps not settable) #28614

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

Open
dfee opened this issue Nov 20, 2018 · 3 comments
Open

Props requiring casting on complex type (and defaultProps not settable) #28614

dfee opened this issue Nov 20, 2018 · 3 comments
Assignees
Labels
Bug A bug in TypeScript
Milestone

Comments

@dfee
Copy link

dfee commented Nov 20, 2018

Search Terms:
defaultProps React.RefForwardingComponent ExoticComponent

Code

// import { cx } from "emotion";
import * as React from "react";

// import modifiers, { ModifierProps } from "../../modifiers";

interface RenderAsExoticComponent<
  TOwnProps,
  TDefaultComponent extends
    | keyof JSX.IntrinsicElements
    | React.ComponentType<any>
>
  extends Pick<
    React.ForwardRefExoticComponent<any>,
    keyof React.ForwardRefExoticComponent<any>
  > {
  (
    props: React.ComponentPropsWithRef<TDefaultComponent> &
      TOwnProps & { renderAs?: never },
  ): JSX.Element | null;
  <TAsComponent extends keyof JSX.IntrinsicElements | React.ComponentType<any>>(
    props: React.ComponentPropsWithRef<TAsComponent> &
      TOwnProps & { renderAs: TAsComponent },
  ): JSX.Element | null;
}

function renderAsComponent<
  TOwnProps,
  TDefaultElement extends React.ComponentType<any> | keyof JSX.IntrinsicElements
>(
  factory: React.RefForwardingComponent<
    any,
    TOwnProps & {
      renderAs?: React.ComponentType<any> | keyof JSX.IntrinsicElements;
      className?: string;
    }
  >,
  defaultElement: TDefaultElement,
) {
  const forward = React.forwardRef(factory);
  forward.defaultProps = { renderAs: defaultElement };
  // todo: apparently a bug, use workaround
  // forward.defaultProps = {};
  // forward.defaultProps.renderAs = defaultElement;
  return forward as RenderAsExoticComponent<TOwnProps, TDefaultElement>;
}

interface ModifierProps {
  textColor?: "white" | "black";
  pull?: "left" | "right";
}

const Element = renderAsComponent<ModifierProps, "div">(
  ({ className, renderAs, ...allProps }, ref) => {
    const props = {
      // className: cx(className, modifiers.classNames(allProps)) || undefined,
      ref,
      // ...modifiers.clean(allProps),
    };
    return React.createElement(renderAs!, props);
  },
  "div",
);

export default Element;
export const Example: React.SFC<{}> = () => (
  <Element textColor="white" pull={"left" as "left"}>
    Child
  </Element>
);

Expected behavior:

  1. defaultProps can be set directly
  2. props work without casting

Actual behavior:

  1. defaultProps cannot be set directly (see workaround in code)
  2. props do not work without being cast.

Playground Link:

Related Issues:
No.

@dfee
Copy link
Author

dfee commented Nov 20, 2018

The error for defaultProps:

Type '{ renderAs: TDefaultElement; }' is not assignable to type 'Partial<PropsWithoutRef<TOwnProps & { renderAs?: "symbol" | "object" | "track" | "progress" | "a" | "abbr" | "address" | "area" | "article" | "aside" | "audio" | "b" | "base" | "bdo" | "blockquote" | ... 157 more ... | undefined; className?: string | undefined; }> & RefAttributes<...>>'.

41   forward.defaultProps = { renderAs: defaultElement };
     ~~~~~~~~~~~~~~~~~~~~

The error for the props casing (i.e. fails on textColor, but succeeds on pull:

[ts] Type 'string' is not assignable to type '"white" | "black" | undefined'. [2322]

@Jessidhia
Copy link

The strange part of the first error is that TDefaultElement is assignable to... that big list of possible values, but it seems the compiler doesn't know that.

@weswigham weswigham added Bug A bug in TypeScript Needs Investigation This issue needs a team member to investigate its status. labels Nov 26, 2018
@weswigham weswigham self-assigned this Dec 8, 2018
@weswigham weswigham added this to the TypeScript 3.3 milestone Dec 8, 2018
@RyanCavanaugh RyanCavanaugh removed this from the TypeScript 3.3 milestone Feb 1, 2019
@RyanCavanaugh RyanCavanaugh removed the Needs Investigation This issue needs a team member to investigate its status. label Mar 7, 2019
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Mar 14, 2019
@weswigham
Copy link
Member

For future me: Here's a non-react minimal repro for the defaultProps bit:

type ForwardRefRenderFunction<T, P> = (props: P) => T;
type ForwardRefExoticComponent<P> = { defaultProps?: Partial<P> };

declare function forwardRef<T, P = {}>(render: ForwardRefRenderFunction<T, P>): ForwardRefExoticComponent<PropsWithoutRef<P>>;

type PropsWithoutRef<P> =
    'ref' extends keyof P
        ? Pick<P, Exclude<keyof P, 'ref'>>
        : P;

function renderAsComponent<
    TOwnProps,
    TDefaultElement
>(
    factory: (x: TOwnProps & {renderAs?: unknown}) => TDefaultElement,
    defaultElement: TDefaultElement,
) {
    const forward = forwardRef(factory);
    forward.defaultProps = { renderAs: defaultElement };
}

(the literal type on tags bit has been fixed since this was opened). The underlying cause twofold.

  • One: Excess property checking relies on the lower bound calculation for mapped types being complete - in this case it is demonstrably not in the presence of generic conditionals. Specifically, we end up with a conditional like Exclude<"renderAs", keyof TProps & "ref"> which ofc, is generic on the extends type and not the check type. This causes it to not simply or contribute to the key bounds in any way (even though, ideally, it'd simplify to "renderAs"), as we only have special-case code in place for giving a lower bound on conditionals which are generic in their check type.
  • Two: Once that's fixed (by any means), the error just moves from the excess property to the assignment itself. The concrete object {renderAs: whatever} cannot be assignable to Partial<SomeGenericConditionalWhereTheResultAlwaysHasRenderAs> under our current conditional type assignability rules, as our conditional type assignability rules are strict - you can't assign a concrete object type to a Partial of a generic conditionally resolved type - you'd need to match the partial and the conditional exactly. To fix that, we'd need proper assignability rules for when the target of an assignment is a conditional (and the source is not), which we have a few PRs for various methods/rules for (eg, Add assignability rules for when the target is a conditional type #27932).

For my future self/others, I have a branch with a potential direction for the first bit; but it's not really useful to fix this without the second bit already in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript
Projects
None yet
Development

No branches or pull requests

4 participants