fix: handle late resolving promises with promise cancelation

[sarahdayan]

This fixes an internal design flaw causing debouncing to malfunction.

TL;DR

The fix uses a custom cancelable promises implementation so we can imperatively prevent pending promises' callback from triggering. The implementation is partial and minimal to keep impact on bundle size as little as possible.

• packages/autocomplete-core/dist/umd/index.production.js: 5.78 KB → 5.97 KB
• packages/autocomplete-js/dist/umd/index.production.js: 16.29 KB → 16.49 KB

Context

Each keystroke triggers onInput, which fetches sources and items, then updates the state:

• adds fetched items to collections
• sets isOpen based on other state values
• etc.

There can be multiple pending requests at the same time, and each request can take an unknown amount of time to settle.

Sometimes, the user can manually close the panel (clicking outside, hitting Escape, hitting Enter) while results are being fetched. In such cases, we don't want the panel to reopen once the requests settle, even if there are results. This was fixed in #829.

current.mov

On blur or Escape, the panel closes and stays closed even when "late" promises settle.

The biggest problem with this implementation is that it doesn't play well with promises than never settle.

Problems

The current design tracks running requests with a counter, then imperatively prevents updates when closing while requests are still running, and only reenables them once they're all settled. Yet in some cases, promises can not settle:

• Our documented debouncePromise function consists of returning a promise that resolves in a timeout, and clearing it when invoking the function before the time is up.
• When a promise is wrongly implemented (e.g., in a way that its callback may not invoke resolve or reject), it could never settle.

In such cases, the request is counted as pending, but will never settle. As soon as the user would close the panel, this situation would cause updates to remain blocked. Conversely, if a request took a really long time, no matter if it was stale and newer requests had already resolved, it would "hold" updates for no reason.

failing-debounce.mov

Another flaw of this design is that it keeps the status loading (see above demo, the loading indicator keeps on running even though we've closed the panel).

Counting running requests the way we do is flawed, and imperatively skipping and reenabling updates is error-prone.

Solution

When the user closes the panel before results are returned, we can consider they're no longer interested. In that case, it's okay not to follow through with updating the state once results are ready: even though it makes sense it the program's flow, it doesn't match the user's behavior and expectations.

In that sense, we should conceptually "drop" any pending request when the user closes the panel. In this context, "dropping" means not executing any attached handler. It's okay for requests to finish, as they will populate the search client's cache. When the user clicks the search box, the state will update with fresh results either from the cache, or wait some more until the request finishes.

Conversely, it should be okay for a promise never to settle. If the user closes the panel, a hanging promise should be garbage collected.

Implementation

The new implementation relies on cancelable promises, a thin promise wrapper around promises that expose a cancel function. When invoking it, any then or catch handler attached to the promise won't be called.

When a request is triggered, its promise is added to a private list of pending requests on the store. This list updates itself: whenever a promise settles or is canceled, it's removed. It also exposes a cancelAll function, which cancels all pending promises and clears the list.

When a user manually closes the panel, cancelAll is invoked, canceling all pending promises and clearing the list. No attached then or catch handler ever triggers, meaning those requests have no impact on the UI state.

This flow is easier to reason with, because it replaces the mental model of arbitrarily skipping state updates with the concept of no longer caring about requests. We no longer need to mix logic in then handlers for when updates are skipped and when they aren't.

We can also immediately make state updates to reflect what's going on (e.g., set the status back to idle, clear the lastStalledId).

cancelable.mov

Debouncing

The solution works well with debouncing, and doesn't affect later state updates as the current implementation does (see #844).

debounce.mov

fixes #844

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 4fc760a:

Sandbox	Source
@algolia/autocomplete-example-github-repositories-custom-plugin	Configuration
@algolia/autocomplete-example-instantsearch	Configuration
@algolia/autocomplete-example-playground	Configuration
@algolia/autocomplete-example-preview-panel-in-modal	Configuration
@algolia/autocomplete-example-react-renderer	Configuration
@algolia/autocomplete-example-starter-algolia	Configuration
@algolia/autocomplete-example-starter	Configuration
@algolia/autocomplete-example-reshape	Configuration
@algolia/autocomplete-example-vue	Configuration
happy-frog-xn5e0	Issue #844

[francoischalifour]

Solid solution!

We can remove some indirections to make the cancelable promise better fit the library and save some bytes.

[Haroenv]

👍

[Haroenv]

lgtm once the "should add be returned" and "can the internals be simplified" comments are addressed :)