babel-plugin-typescript-to-proptypes icon indicating copy to clipboard operation
babel-plugin-typescript-to-proptypes copied to clipboard

Incorrect code generated for union types

Open ianhoffman opened this issue 3 years ago • 2 comments

Description

Given a union like type C = A | B, where A and B are both interfaces, babel-plugin-typescript-to-prototype generates code requiring that C contain all values present in A and B, rather than values present in either A or B. For example:

import React from 'react';

interface A {
    foo: string;
}

interface B {
    bar: string;
}

type C = A | B;

export const Component = (_props: C) => {
    return <React.Fragment></React.Fragment>;
}

This will generate the following code:

var Component = function Component(_props) {
  return /*#__PURE__*/_react["default"].createElement(_react["default"].Fragment, null);
};

exports.Component = Component;
Component.propTypes = {
  foo: _propTypes["default"].string.isRequired,
  bar: _propTypes["default"].string.isRequired
};

For a full repro, see https://github.com/ianhoffman/union-bug-repro

Expected Behavior

Given the above example, we should generate something like the following:

var Component = function Component(_props) {
  return /*#__PURE__*/_react["default"].createElement(_react["default"].Fragment, null);
};

exports.Component = Component;
Component.propTypes = _propTypes["default"].oneOfType([
    _propTypes["default"].shape({
        foo: _propTypes["default"].string.isRequired,
    }),
    _propTypes["default"].shape({
        bar: _propTypes["default"].string.isRequired
    }),
]).isRequired;

ianhoffman avatar Mar 18 '22 17:03 ianhoffman

@ianhoffman This doesn't support props as a union, since when extracted, the prop types are simply a list of all possible fields: https://github.com/milesj/babel-plugin-typescript-to-proptypes/blob/master/src/addToClass.ts#L36

I don't really maintain this anymore, so I won't be fixing this, but feel free to submit a PR.

milesj avatar Mar 18 '22 18:03 milesj

Thanks for the quick response. This looks like it'll require a pretty fundamental change to the way the plugin generates prop types (at least if we want to support things beyond the simplest case, e.g., A & (B | C)). I'm probably going to table working on it.

ianhoffman avatar Mar 18 '22 23:03 ianhoffman