Skip to content
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

Don't warn in react/jsx-props-no-spreading when all properties are listed explicitly. #2439

Closed
MattSmiglarski opened this issue Oct 3, 2019 · 11 comments · Fixed by #2449
Closed

Comments

@MattSmiglarski
Copy link

A syntactic workaround for avoiding repetition in the following JSX:

<Foo prop1={prop1} prop2={prop2} prop3={prop3} />

..is to instead write:

<Foo {... { prop1, props2, prop3 }} />

The reason for react/jsx-props-no-spreading is to encourage being explicit with properties, while allowing exceptions to be configured for Higher Ordered Components. The above example is not a HOC, but is explicit about the properties.

Therefore, there should be a configuration to allow this case ("spreading of an inline object literal that does not itself contain any spreads"), perhaps even as a default.

That said, ideally JSX would provide a nicer non-repetitive syntax for the above.

@ljharb
Copy link
Member

ljharb commented Oct 3, 2019

An option to allow this case sounds great, and does not conflict with the impetus for the rule - that implicit spreading is an antipattern.

@pawelnvk
Copy link
Contributor

pawelnvk commented Oct 5, 2019

I added #2449, that might help here. I thought for a while about allowing also to handle cases when some given props are assigned to dedicated variable and than spread, e.g.:

const modalProps = { prop1, prop2, prop3 };

return <Modal {...modalProps } />

I feel that this could be helpful, yet current implementation is good enough to solve your problem without extending implementation complexity by far. What do you think guys?

@trevtrich
Copy link

trevtrich commented Oct 8, 2019

We're currently using Downshift and it's api provides render props that are things like getInputProps to spread into the input element. Example:

<Downshift
    onChange={selection =>
      alert(selection ? `You selected ${selection.value}` : 'Selection Cleared')
    }
    itemToString={item => (item ? item.value : '')}
  >
    {({
      getInputProps,
      isOpen,
      inputValue
    }) => (
      <div>
        <input {...getInputProps()} />

Not sure how tricky it would be to pickup on cases like this, but thought this might add another element to the conversation here.

@ljharb
Copy link
Member

ljharb commented Oct 8, 2019

@richardson-trevor there's no good way to pick up on that, and it's debatable whether that's a good pattern anyways. For that case, use an eslint override comment.

@ljharb ljharb closed this as completed in 0877bcd Oct 8, 2019
@bzamora
Copy link

bzamora commented Nov 15, 2019

const modalProps = { prop1, prop2, prop3 };

return <Modal {...modalProps } />

Allowing this would be really useful

@ljharb
Copy link
Member

ljharb commented Nov 15, 2019

@bzamora it's done in #2449.

@bzamora
Copy link

bzamora commented Nov 19, 2019

@ljharb reading upstream, it seems scenario of using a dedicated variable and then spread isn't covered? If it is, then 👍

See #2439 (comment)

@ljharb
Copy link
Member

ljharb commented Nov 19, 2019

No, it's not, but that would only even be possible to handle if the variable is defined in the same file.

I'd be open to a PR exploring that, but this issue should stay closed, as it's resolved.

stefan-wullems pushed a commit to stefan-wullems/eslint-plugin-react that referenced this issue Nov 30, 2019
@arathjz
Copy link

arathjz commented Apr 7, 2020

Does anyone followed up this scenario?

const modalProps = { prop1, prop2, prop3 };

return <Modal {...modalProps } />

@ljharb
Copy link
Member

ljharb commented Apr 7, 2020

@arathjz yes, see #2449 - there's been no followup since.

@arathjz
Copy link

arathjz commented Apr 7, 2020

@ljharb it seems #2449 does not cover this yet

const modalProps = { prop1, prop2, prop3 };

return <Modal {...modalProps } />

it only covers explicit props as

return <Modal {...{ prop1, prop2, prop3 }} />

According to #2249 tests that case is still displaying:Prop spreading is forbidden

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

6 participants