withDefaults React HOC and undefined input props

[OliverJAsh]

The documentation suggests a withDefaults React HOC. We've been using this in production and we love it, but it has an important issue which I'd like to note for others, and potentially fix with updated docs.

TypeScript does not distinguish optional properties (e.g. values either missing or provided with a given type) from undefined: microsoft/TypeScript#13195

This means that an undefined prop may be passed to a component whose prop is marked as optional—not undefined—and this will override the default prop: microsoft/TypeScript#13195 (comment)

type Props = {
    foo: string;
    bar: string
}

type InputProps = {
    foo?: string;
    bar: string;
}

const defaultProps: Pick<Props, 'foo'> = { foo: 'foo' };
const inputProps: InputProps = { foo: undefined, bar: 'bar' };

// Type of `foo` property is `string` but actual value is `undefined`
const completeProps: Props = { ...defaultProps, ...inputProps };

Ideally, TypeScript would omit an error when trying to pass undefined to an optional prop:

// `{ foo: undefined; bar: string; }` is not assignable to `{ foo?: string; bar: string; }`
const inputProps: InputProps = { foo: undefined, bar: 'bar' };

In our experience, this has lead the several bugs. The types do not match the values at runtime. A component may receive a prop with a value of undefined, even though undefined is not in that prop's type.

In the meantime, I think withDefaults should allow default props to take precedence over input props when the input props are undefined (either because they aren't passed through at all, or they were passed through but as undefined). This matches the behaviour of React's built-in defaultProps static property on components: https://github.com/facebook/react/blob/373a33f9d30d02cfea814a7803209da6f860a252/packages/react/src/ReactElement.js#L224

Here is the updated version we've been using. Let me know what you think and if you'd like to update the docs to include these modifications:

import isUndefined from 'lodash/isUndefined';
import omitBy from 'lodash/omitBy';
import React from 'react';
import { ObjectDiff } from 'typelevel-ts';

import { getDisplayName } from 'HOCs/helpers';

export const withDefaults = function<D, A extends D> (
  C: React.ComponentType<A>,
  defaults: D,
): React.SFC<ObjectDiff<A, D>> {
  const displayName = `WithDefaults(${getDisplayName(C)})`;
  const WithDefaults: React.SFC<ObjectDiff<A, D>> = (props) => {
    // If a prop is provided with a value of `undefined`, we want the default prop to take
    // precendence. This ensures the behaviour matches that of React's built-in `defaultProps`
    // static property on components.
    const propsMinusUndefined = omitBy(props, isUndefined);
    return <C {...defaults} {...propsMinusUndefined} />;
  };
  WithDefaults.displayName = displayName;
  return WithDefaults;
};

[gcanti]

Let me know what you think and if you'd like to update the docs to include these modifications

Sure, thanks for pointing out