Make inputs_for keyed #3884
Make inputs_for keyed #3884
Conversation
|
To have this make a difference, we need to also think about improving the |
| ~H""" | ||
| <input | ||
| :for={{name, value, index} <- @hidden_inputs} | ||
| :key={"#{name}_#{index}"} |
There was a problem hiding this comment.
Using index as part of the key is almost certainly a bug waiting to happen.
I'm assuming you're avoiding using just name as key since that might not be unique, but adding index doesn't fix this, as it just leads to different kind of bugs when name isn't unique.
I've seen this many times in react, those bugs are very hard to find.
(I haven't properly studied this PR or the implementation of :key so ignore me if you definitely know better. Just something that jumped out to me)
There was a problem hiding this comment.
Names are indeed not unique. myinput[] is a totally valid name to be used multiple times.
But I'm curious what you think the "bugs waiting to happen" would be. Keying is an optimization, which in the worst case would only fall back to being unoptimized like inputs_for is right now already.
There was a problem hiding this comment.
I don't know how :key is implemented in 1.1, but for react you introduce bugs when key is not uniquely identifying the same element between renders. I would be very surprised if the implementation of liveview avoids these issues completely.
The bugs are subtle and only happen when you reorder or add/remove elements. Here's an old article for react: https://robinpokorny.com/blog/index-as-a-key-is-an-anti-pattern/
example from the blog: https://jsbin.com/wohima/edit?output
For the concrete code here it's certainly harder to run into this issue, but if it's possible in principle I guarantee a user will run into this issue by accident, and then tracking it down will be almost impossible.
3 participants
No description provided.