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

Enhancement: added option checkKeyMustBeforeSpread of react/jsx-key #2830

Closed
morlay opened this issue Oct 16, 2020 · 6 comments · Fixed by #2835
Closed

Enhancement: added option checkKeyMustBeforeSpread of react/jsx-key #2830

morlay opened this issue Oct 16, 2020 · 6 comments · Fixed by #2835

Comments

@morlay
Copy link
Contributor

morlay commented Oct 16, 2020

jsx-runtime landed. but there have different transform results when key before or after props spread

facebook/react#20031

<div key="key "{...props} />;

<div {...props} key="key"/>;

// will convert to 

import { createElement } from "react"
import { jsx } from "react/jsx-runtime"

jsx("div", props, "key")
createElement("div", { ...props, key: "key" }) // only key after props spread do the fallback 

to avoid the fallback to using createElement,
need a rule to limit the key before spread.

@ljharb
Copy link
Member

ljharb commented Oct 16, 2020

facebook/react#20031 (comment) suggests the opposite - that the key must be after the spread so react can statically know which one will "win".

@ljharb
Copy link
Member

ljharb commented Oct 16, 2020

Let's wait until that issue gets some resolution before implementing anything here.

@morlay
Copy link
Contributor Author

morlay commented Oct 16, 2020

@ljharb Yes. we need waiting. Just put the issue here for tracking.

babel and typescript already have implemented this logic. So i'm not optimistic to they will change it.

and typescript and babel have diffrent fallback results.

https://www.typescriptlang.org/play?ts=4.1.0-dev.20201015#code/PQKhAIAECsGcA8CSBbADgewE4BcDK6BXTAYwFMpTl1sBLdAO2E1IENjtwRgAobgHlioW9cAG8AdJNSZ0qWAF9wwAHy8BQkQGtSATwC8AIhYGxk8dNkKlq-oOGmpMuYu36DAExawTKoA

typescript is not use createElement of @jsxImportSource, but React.createElement direclty.

if nothing changed. This eslint rule is really needed.

@ljharb
Copy link
Member

ljharb commented Oct 16, 2020

Sounds like we'll want to make a rule that warns when the key appears after the spread, despite that being highly counterintuitive.

@morlay
Copy link
Contributor Author

morlay commented Oct 17, 2020

seems this may a temporary rule until react evolution completed.

maybe as a option of react/jsx-key

@ljharb
Copy link
Member

ljharb commented Oct 17, 2020

I like that better.

@morlay morlay changed the title New Rule: key-must-before-spread Enhancement: added option checkKeyMustBeforeSpread of react/jsx-key Oct 19, 2020
@ljharb ljharb closed this as completed in 9f0d5c4 Oct 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants