facebook / prop-types

Runtime type checking for React props and similar objects
MIT License
4.48k stars 358 forks source link

Importing a reference to PropTypes as opposed Proptypes directly #391

Closed dhmcloud closed 1 year ago

dhmcloud commented 1 year ago

Hey, I'm just wondering why I can't use

import PropTypes from "prop-types";

export const PT = PropTypes;

and then

Timestamp.propTypes = {
  isoTime: PT.string.isRequired,
  longFormat: PT.bool,
  verbiage: PT.string,
  side: PT.oneOf(["left", "right", "top", "bottom"]),
  titleCase: PT.bool,
};

The issue I see is when I use React Storybook, it looks like PropTypes works but PT won't work...

ljharb commented 1 year ago

Of course you can, because that's how JS works - but it'd be much less clear to readers of your code, which is why it's unadvisable.

dhmcloud commented 1 year ago

@ljharb Okay, that's what I was using, but, when I use it with storybook, the controls get all messed up... does that make sense?

dhmcloud commented 1 year ago

@ljharb

Example Story:

import { EXAMPLE_DATETIME_ISO } from "~/fixtures";

import { Timestamp } from "./Timestamp";

export default {
  component: Timestamp,
  title: "Components/Timestamp",
};

export const Normal = {
  args: {
    isoTime: EXAMPLE_DATETIME_ISO,
    longFormat: true,
    side: "left",
    titleCase: false,
    verbiage: "",
  },
};

Example Proptypes using PT:

Timestamp.propTypes = {
  isoTime: PT.string.isRequired,
  longFormat: PT.bool,
  side: PT.oneOf(["left", "right", "top", "bottom"]),
  titleCase: PT.bool,
  verbiage: PT.string,
};

Result:

Screen Shot 2023-01-11 at 1 22 44 PM

Using PropTypes

Timestamp.propTypes = {
  isoTime: PT.string.isRequired,
  longFormat: PT.bool,
  side: PropTypes.oneOf(["left", "right", "top", "bottom"]), <- - - - 
  titleCase: PT.bool,
  verbiage: PT.string,
};

Result:

Screen Shot 2023-01-11 at 1 24 25 PM
ljharb commented 1 year ago

I believe that's because Storybook does some light static analysis to determine if something is an official PropTypes thing or not.

Either way, I'd strongly suggest not doing this - re-exporting things only causes problems.