feat: Update Selected options if options changed #1098
Conversation
JonnCharpentier
requested review from
stephenh,
ksk5280 and
chr1sjf0x
as code owners
| const selectedOptions = useMemo(() => { | ||
| // `selectedOptions` should only ever update if the `values` prop actually change. | ||
| // Assuming `values` is a state variable, then it should hold its identity until it _really_ changes. | ||
| // Though, it is possible that the `options` prop has changed, which is a dependency on this `useMemo`. | ||
| // That could trigger an unnecessary new reference for `selectedOptions`, and cause additional renders or unexpected state changes. | ||
| // We should avoid updating `selectedOptions` unless `values` has actually changed. | ||
| if (!equal([...values].sort(), selectedOptionsRef.current.map(getOptionValue).sort())) { | ||
| selectedOptionsRef.current = options.filter((o) => values.includes(getOptionValue(o))); | ||
| } |
There was a problem hiding this comment.
concern: I'm a little worried that this big comment was added for good reason, and that removing this code will regress the issue (especially since it is going back to the same implementation as before)
I believe due diligence here is required to figure out:
- What functionality does this fix, and is it still an issue in the system/regress when removed?
- If it is an issue, can we update this in a way that fixes your issue and keeps the other issue from reappearing?
There was a problem hiding this comment.
The issue this is meant to fix this: https://homeboundinc.slack.com/archives/G01QEA5CXL1/p1714491658184339, not sure if is still a problem
There was a problem hiding this comment.
Per slack discussion, this PR doesn't fully revert Brandon's fix, which also had a "if debounce search then early return" removed, that is staying removed.
I'm pretty willing to bet the "early return" was the core/biggest fix, and that this "set a useRef from within a useMemo" was ...probably... not necessary, given Jonn's retested that page and it seems to still be working fine.
That said, if not, we'll take another crack at this, and likely with a "something different than setting a ref from within a use memo" approach.
## [2.376.0](v2.375.0...v2.376.0) (2024-12-17) ### Features * Update Selected options if options changed ([#1098](#1098)) ([d2d89b4](d2d89b4))
|
🎉 This PR is included in version 2.376.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
No description provided.