async-select-refactor #3372
Closed
async-select-refactor #3372
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gwyneplaine
force-pushed
the
async-select-refactor
branch
from
00b5374 to
3996f29
Compare
jossmac
reviewed
Feb 14, 2019
| const promiseOptions = inputValue => | ||
| new Promise(resolve => { | ||
| const promiseOptions = inputValue => { | ||
| return new Promise(resolve => { |
There was a problem hiding this comment.
Is this change just for readability?
| constructor(props: Props) { | ||
| super(); | ||
| super(props); |
gwyneplaine
force-pushed
the
async-select-refactor
branch
from
3996f29 to
c2ecc1d
Compare
gwyneplaine
force-pushed
the
async-select-refactor
branch
from
c2ecc1d to
b2cbdaa
Compare
gwyneplaine
force-pushed
the
v3.0.0
branch
3 times, most recently
from
955232b to
abbe492
Compare
|
I'm not really into this project, but this PR looks good to me. |
bladey
added
pr/needs-review
bladey
added
pr/enhancement
|
@Methuselah96 thoughts on closing this in light of the refactor to TypeScript? |
|
This is significantly out of date since |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Async Select Refactor
Overview
Previous Async Select was a composition of makeAsyncSelect, manageState and the base Select component in the following order.
makeAsyncSelect(manageState(Select));This meant the async logic from the exported Async Select was not able to leverage the helpful abstraction of controlled properties like onChange and inputValue that manageState provides.
This PR aims to rectify this by changing this composition order, to the following:
manageState(makeAsyncSelect(Select))In doing so we are afforded a bunch of niceties:
AFAIK the api also doesn't change, so this might even be possible to ship as either 2.3.1 or 2.4.0 @JedWatson