feat: Add SelectField.mapOption prop. #195
Draft
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Trying out an alternative API for
SelectField
.Having to type out
getOptionLabel
andgetOptionValue
all the time has been boilerplately.Our current attempt to reduce this was some type system magic to realize "oh the option already has an id+name, we'll just use that", and then make
getOptionLabel
andgetOptionValue
optional for just that one specific type of option.But it was pretty ugly to pull off, and also very limited, you had to implement id+name to get the succinct versions.
Playing around, it seems cute to combine
getOptionLabel
+getOptionValue
+getOptionMenuLabel
into a single prop of "hey we need you to map this option to 'stuff we need'".The upshot of it being a single prop is that, even when defining by hand it's more succinct, but also you can apply defaults for all three behaviors from a single function, i.e.
idAndName
:The one downside is that, if you did have
id+name
now you have the extra boilerplate of passingmapOption={idAndName}
... however, I think the majority of our callers didn't magically haveid+name
anyway, and if anything this will be easier to make moreidAndName
-ish functions for common combinations i.e.idAndLabel
or what not.Opening this up to kinda think about. So far I'm bullish that this is cleaner/better, but not going to rush merging it.