react-querybuilder
the key used when iterating on rule list is causing state issues
Summary
Hello! I am using the library alongside with custom controlElements and in my implementation of valueEditor, I'm facing state issues that seem to be caused by non-optimal use of the key property during iterations.
Description
Let's say I have two rules, rule#1 and rule#2. When I delete the rule#1 thanks to the Remove rule button, it appears that React tries to use the UI instance of the deleted rule#1 to put the rule#2's content into it (e.g. all the props including field, operator, value..). To illustrate that, in the default ValueEditor, I've tested to add console.log of the operator and of the previous render's operator, and here is what's happening when deleting the first rule:
=> React seems to be trying to fill the Last name in rule's content into the instance of the former First name begins with rule.
After doing some digging into the library's source code, it appears in RuleGroup.tsx#line304 that when iterating on rules, you use indexes as the key to repeated elements.
React recommends to use unique identifiers as keys: indeed, if the order can change, using indexes "can negatively impact performance and may cause issues with component state", and that's precisely the problem I'm encountering.
Suggestion
Would that be possible for you to use unique identifiers for this key, such as the rule's id for example?
Thank you so much for the detailed report! My thought process while reading this was something like, "Well ackshually it uses a string representation of the path, which is a stabl...wait...oh no...the path is just the group path plus the index...which is NOT STABLE! How did my life get to this point?!"
Seriously though, I can't believe that issue hasn't been caught until now. I'll fix it in v5.0 which I'm working on releasing as fast as I can. Your suggestion to use the id is the correct fix. Thanks again!
Thank you for your quick reply.
No problem, happy to help!
Until I can upgrade to the upcoming v5, in my client project, I am going to override the RuleGroup of the controlElements with a copy of the default one, but with having set the Fragment's key to the rule's id. That will solve my state issues for now, until the v5 comes out and I'll remove it!
Sounds good. I may release a v4.5.3 with this fixed if I get time.
Also, copying the RuleGroup component should work but I'd recommend using patch-package instead.
@jbailleul I just released v4.5.3 🎉
(This issue will remain open until #370 is merged into main.)