From d0d039637af9d404aa47fd1f9c737bf5e3ca94d8 Mon Sep 17 00:00:00 2001
From: Timothy Hwang <thwang@rocketfuel.com>
Date: Mon, 18 Sep 2017 13:51:11 -0400
Subject: [PATCH 1/2] Async> cache async response regardless of req order

Addresses #2009.

There is a WIP test -- I can't figure out how to simulate the exact
scenario that this patch fixes in the test. The current test returns
positive even without my change. Suggestions would be appreciated --
thanks!
---
 src/Async.js       | 12 ++++++------
 test/Async-test.js | 28 ++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/src/Async.js b/src/Async.js
index b19df77f1f..48dc1ec69e 100644
--- a/src/Async.js
+++ b/src/Async.js
@@ -96,14 +96,14 @@ export default class Async extends Component {
 		}
 
 		const callback = (error, data) => {
-			if (callback === this._callback) {
-				this._callback = null;
+			const options = data && data.options || [];
 
-				const options = data && data.options || [];
+			if (cache) {
+				cache[inputValue] = options;
+			}
 
-				if (cache) {
-					cache[inputValue] = options;
-				}
+			if (callback === this._callback) {
+				this._callback = null;
 
 				this.setState({
 					isLoading: false,
diff --git a/test/Async-test.js b/test/Async-test.js
index 3e76e9fc15..4c28d1395d 100644
--- a/test/Async-test.js
+++ b/test/Async-test.js
@@ -146,6 +146,34 @@ describe('Async', () => {
 			return expect(asyncNode.textContent, 'to contain', 'Loading');
 		});
 
+		// TODO: I'm not sure what the best way to test this is, because this test returns positive even
+		// before the change. However, manual testing by throttling the network with Chrome devtools shows
+		// that this fixes the issue.
+		it.skip('caches the result of all option fetches', () => {
+			const optionResponse = createOptionsResponse(['foo']);
+			function loadOptions (input, resolve) {
+				setTimeout(function() {
+					resolve(null, optionResponse);
+				}, 10 - input.length);
+				// resolve(null, optionResponse);
+			}
+			createControl({
+				loadOptions,
+			});
+			const instance = asyncInstance;
+			typeSearchText('t');
+			typeSearchText('te');
+			typeSearchText('tes');
+
+			// TODO: How to test this?
+			setTimeout(function() {
+				console.log(instance._cache);
+				expect(instance._cache.t, 'to equal', optionResponse.options);
+				expect(instance._cache.te, 'to equal', optionResponse.options);
+				expect(instance._cache.tes, 'to equal', optionResponse.options);
+			}, 30);
+		});
+
 		describe('with callbacks', () => {
 			it('should display the loaded options', () => {
 				function loadOptions (input, resolve) {

From 79f478e70a1d837962fa4a59c2597fbf1816ee62 Mon Sep 17 00:00:00 2001
From: Timothy Hwang <thwang@rocketfuel.com>
Date: Mon, 18 Sep 2017 14:16:31 -0400
Subject: [PATCH 2/2] Clear this._callback on cache hit

Addresses the second issue mentioned in the linked issue. While the
previous commit makes this change unnecessary for regular use cases, the
issue is still present when debouncing. By clearing out
`this._callback`, intermediary searches (e.g. 'Ti' when 'Tim' is already
fetched) won't cause the options for 'Tim' to change when the request
for 'Ti' completes.
---
 src/Async.js | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/Async.js b/src/Async.js
index 48dc1ec69e..2d9fc7d7ae 100644
--- a/src/Async.js
+++ b/src/Async.js
@@ -88,6 +88,8 @@ export default class Async extends Component {
 			cache &&
 			Object.prototype.hasOwnProperty.call(cache, inputValue)
 		) {
+			this._callback = null;
+
 			this.setState({
 				options: cache[inputValue]
 			});