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

Key prop and keyed function #124

Open
ghivert opened this issue Jul 2, 2020 · 3 comments
Open

Key prop and keyed function #124

ghivert opened this issue Jul 2, 2020 · 3 comments

Comments

@ghivert
Copy link

ghivert commented Jul 2, 2020

Hi!

I was wondering, is there any way to activate a warning when not using a key when mapping over a collection? Usually React print it like "…bla bla bla use a key…", and it’s sometimes boring, but in the end, it’s simpler to figure out where a key is missing.

If not, maybe I can try to implement it in a PR?

@megamaddu
Copy link
Member

megamaddu commented Jul 2, 2020

If you have any ideas on how to do it that'd be great! Unfortunately the way React does this doesn't really translate to PS. In JS/JSX, elements passed as extra jsx arguments skip the warning, while elements passed as an array (or nested arrays) warn:

React.createElement("div", { children: [thisWarns] }, thisDoesnt, neitherDoesThis, [but, these, do], and, these, dont)

But in PS we have ReactComponent { children :: Array JSX }, which essentially forces everything into the warning case. Since we really don't want to spew thousands of warnings we instead use createElement.apply(null, ["div", props].concat(props.children)).

It's possible an alternate api would be able to handle this better, using some concept of "Renderable" where both JSX and Array JSX are renderable. That along with do-notation could make that possible, but it might also introduce performance issues in large apps.

@ghivert
Copy link
Author

ghivert commented Jul 15, 2020

Sorry for the long response delay.


It's possible an alternate api would be able to handle this better, using some concept of "Renderable" where both JSX and Array JSX are renderable. That along with do-notation could make that possible, but it might also introduce performance issues in large apps.

For sure, but does it really help for code readability and maintenance? Because it adds another layer, etc. Here, the api is really simple and it’s cool: just give an array of children and that’s fine. I’m not sure the tradeoff is worth it. 😕


I didn’t know how React is handling that, and it seems hard to translate this in PS. I can try to get a Renderable anyway, to see how it can be achieved in PS. Maybe that way we can explore and push a little further? Unfortunately I’m taking holidays the next two weeks, but I can try it when I’m back.

@megamaddu
Copy link
Member

No worries, I don't know that it's worth it either. If you feel like taking a stab at it that's cool, but I don't think there's much need for it.

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

No branches or pull requests

2 participants