fix: Refactor ComboBoxBase to memo more #964
Conversation
stephenh
force-pushed
the
refactor-combo-state
branch
2 times, most recently
from
e82ca05 to
26f37c1
Compare
| const r = await render( | ||
| <SelectField | ||
| label="label" | ||
| value={true} |
There was a problem hiding this comment.
Before this refactoring, this value could stay hard-coded and ComboBoxBase had its own internal version of values that could drift from the value prop.
But now, kinda on accident but I think it makes sense, the internal value is always derived from the value prop, so it behaves more like a controlled component.
Which is fine, afaict not a breaking change, but meant these tests that were hard-coding value=true needed to change.
| ), | ||
| filteredOptions: options, | ||
| allOptions: options, | ||
| selectedOptions, |
There was a problem hiding this comment.
filteredOptiosn, allOptions, and selectedOptions, and selectedKeys are now all derived from the useMemo-d options / values or what not.
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| [options], | ||
| ); | ||
| // Reset inputValue when closed or selected changes |
There was a problem hiding this comment.
Kinda surprised, this useEffect ends up being a pretty cute place to handle resetting inputValue as various conditions change.
stephenh
force-pushed
the
refactor-combo-state
branch
from
26f37c1 to
35ef286
Compare
| // We need separate `searchValue` vs. `inputValue` b/c we might be showing the | ||
| // currently-loaded option in the input, without the user having typed a filter yet. | ||
| searchValue: string | undefined; |
There was a problem hiding this comment.
praise: Nice, that does help keep this a bit cleaner
## [2.321.1](v2.321.0...v2.321.1) (2023-10-12) ### Bug Fixes * Refactor ComboBoxBase to memo more ([#964](#964)) ([f1329f6](f1329f6))
|
🎉 This PR is included in version 2.321.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Marking this as a "fix" to get it released, but it's really a chore/no change refactoring.
After working on the "current/load/options" refactoring, I pulled the thread a little bit on useMemo-ing things like
getOptionLabel,getOptionValue.And then also noticed that, AFAICT, ~3-4 keys in the
FieldStatethat were manually set in multiple places could, again AFAICT, be derived with useMemos.So that's what this does.