Skip to content
This repository has been archived by the owner on Sep 1, 2024. It is now read-only.

Cannot represent spread ...props passed to a child #319

Closed
jamesarosen opened this issue Jul 6, 2020 · 3 comments
Closed

Cannot represent spread ...props passed to a child #319

jamesarosen opened this issue Jul 6, 2020 · 3 comments
Labels

Comments

@jamesarosen
Copy link

jamesarosen commented Jul 6, 2020

Let's say there's a third-party library like this:

export default function Notice({ header, body, className }) {
  return ()
}

Notice.defaultProps = {
  className: '',
}

Notice.propTypes = {
  header: PropTypes.string.isRequired,
  body: PropTypes.string.isRequired,
  className: PropTypes.string,
}

And I want to wrap it:

export default function MyNotice({ type, ...props }) {
  return <Notice {{...props, className: `my-notice--${type}`}} />
}

MyNotice.defaultProps = {
  type: 'info',
}

MyNotice.propTypes = {
  type: PropTypes.oneOf([ 'debug', 'info', 'warn', 'error' ]),
}

If I don't declare header, body, and className as props on MyNotice then I get warnings about unused props.

If I do declare them on MyNotice, then I get a warning from react/no-unknown-property. (I can disable that warning, but it seems like its existence suggests that the team thinks all props should be declared.)

I can try to merge in the child component's prop types:

MyNotice.propTypes = {
  ...Notice.propTypes,
  type: PropTypes.oneOf([ 'debug', 'info', 'warn', 'error' ]),
}

But that gets a warning from react/forbid-foreign-prop-types. And even if it didn't, many third-party libraries strip the prop-types from their exported builds.

I'd love to be able to do something like this:

MyNotice.propTypes = {
  type: PropTypes.oneOf([ 'debug', 'info', 'warn', 'error' ]),
  ...PropTypes.spread,
}

Related: #285

@jamesarosen jamesarosen changed the title Cannot represent spread ...props passed to a child? Cannot represent spread ...props passed to a child Jul 6, 2020
@ljharb
Copy link
Contributor

ljharb commented Jul 6, 2020

forbid-foreign-prop-types ensures that you use an explicitly exported propTypes shape, and not the .propTypes property; any explicitly exported propTypes wouldn't be stripped by any build.

Since you're trying to deal with linter warnings, you should file this on eslint-plugin-react, not here - there's nothing this project can do or needs to do to address your question.

(However, this is also intentional; you should be either manually duplicating Notice's propTypes on MyNotice, or, explicitly exporting the propTypes object so you can spread that into MyNotice.propTypes and then disabling the relevant eslint warnings)

@ljharb ljharb closed this as completed Jul 6, 2020
@ljharb ljharb added the question label Jul 6, 2020
@jamesarosen
Copy link
Author

jamesarosen commented Jul 6, 2020

you should be either manually duplicating Notice's propTypes on MyNotice

If I declare the props and pass them explicitly to the child then I won't get Notice's defaults.

If I declare the props but then pass them as ...props then I get a lint warning about unused props. I guess I can take that to the other project.

or, explicitly exporting the propTypes object so you can spread that into MyNotice.propTypes and then disabling the relevant eslint warnings

That assumes I have control over the third-party package. I don't think it's fair to assume that all React packages that use PropTypes export their types as standalone constants.

A completely non-scientific quick survey:

@ljharb
Copy link
Contributor

ljharb commented Jul 6, 2020

I agree, but you could make a PR. Either way, these are linter warnings - if they don't make sense for an edge case, use an override comment and explain why you overrode it :-)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants