Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add runAsync() directive #657

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 3 additions & 2 deletions CHANGELOG.md
Expand Up @@ -10,9 +10,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
Unreleased section, uncommenting the header as necessary.
-->

<!-- ## Unreleased -->
## Unreleased
<!-- ### Changed -->
<!-- ### Added -->
### Added
* Added `runAsync()` directve to run async tasks and render templates based on the task state. ([#657](https://github.com/Polymer/lit-html/pull/657))
<!-- ### Removed -->
<!-- ### Fixed -->

Expand Down
31 changes: 31 additions & 0 deletions docs/_guide/05-template-reference.md
Expand Up @@ -260,6 +260,7 @@ lit-html includes a few built-in directives.
* [`ifDefined`](#ifdefined)
* [`guard`](#guard)
* [`repeat`](#repeat)
* [`runAsync`](#runasync)
* [`styleMap`](#stylemap)
* [`unsafeHTML`](#unsafehtml)
* [`until`](#until)
Expand Down Expand Up @@ -473,6 +474,36 @@ html`<p style=${styleMap(styles}>Hello style!</p>`;

For CSS properties that contain dashes, you can either use the camel-case equivalent, or put the property name in quotes. For example, you can write the the CSS property `font-family` as either `fontFamily` or `'font-family'`:

### runAsync

`runAsync(key, task, templates)`

Runs an async function whenever the key changes, and calls one of several
lit-html template functions depending on the state of the async call:

- success() is called when the result of the function resolves.
- pending() is called immediately
- initial() is called if the function rejects with a InitialStateError,
which lets the function indicate that it couldn't proceed with the
provided key. This is usually the case when there isn't data to load.
- failure() is called if the function rejects.

Example:

```js
import { runAsync } from 'lit-html/directives/run-async.js';

const template(url) => html`${
runAsync(url, (url) => fetch(url).then((r) => r.text()), {
success: (content) => html`Success: ${content}`,
pending: () => 'Pending',
initial: () => `Initial`,
failure: (e: Error) => `Error: ${e.message}`
})}`
```

### asyncAppend and asyncReplace

```js
{ fontFamily: 'roboto' }
{ 'font-family': 'roboto' }
Expand Down
153 changes: 153 additions & 0 deletions src/directives/run-async.ts
@@ -0,0 +1,153 @@
/**
* @license
* Copyright (c) 2018 The Polymer Project Authors. All rights reserved.
* This code may only be used under the BSD style license found at
* http://polymer.github.io/LICENSE.txt
* The complete set of authors may be found at
* http://polymer.github.io/AUTHORS.txt
* The complete set of contributors may be found at
* http://polymer.github.io/CONTRIBUTORS.txt
* Code distributed by Google as part of the polymer project is also
* subject to an additional IP rights grant found at
* http://polymer.github.io/PATENTS.txt
*/

import {directive, NodePart, Part} from '../lit-html.js';

interface AsyncRunState {
key: unknown;
promise: Promise<unknown>;
abortController?: AbortController;
state: 'initial'|'pending'|'success'|'failure';
resolvePending: () => void;
rejectPending: (e: Error) => void;
}

const hasAbortController = typeof AbortController === 'function';

const runs = new WeakMap<Part, AsyncRunState>();

export type Task<K> = (key: K, options: {signal?: AbortSignal}) =>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed, key (singular) should be changed to dependencies (plural, array)

Promise<unknown>;

/**
* Runs an async function whenever the key changes, and calls one of several
* lit-html template functions depending on the state of the async call:
*
* - success() is called when the result of the function resolves.
* - pending() is called immediately
* - initial() is called if the function rejects with a InitialStateError,
* which lets the function indicate that it couldn't proceed with the
* provided key. This is usually the case when there isn't data to load.
* - failure() is called if the function rejects.
*
* @param key A parameter passed to the task function. The task function is only
* called when they key changes.
* @param task An async function to run when the key changes
* @param templates The templates to render for each state of the task
*/
export const runAsync = directive(<K>(key: K, task: Task<K>, templates: {
success: (result: any) => any,
pending?: () => any,
initial?: () => any,
failure?: (e: Error) => any
}) => (part: NodePart) => {
const {success, pending, initial, failure} = templates;
const currentRunState = runs.get(part);

// The first time we see a value we save and await the work function.
// TODO(justinfagnani): allow a custom invalidate function
if (currentRunState === undefined || currentRunState.key !== key) {
// Abort a pending request if there is one
if (currentRunState !== undefined && currentRunState.state === 'pending') {
if (currentRunState.abortController !== undefined) {
currentRunState.abortController.abort();
}
// TODO(justinfagnani): This should be an AbortError, but it's not
// implemented yet
currentRunState.rejectPending(new Error());
}
const abortController =
hasAbortController ? new AbortController() : undefined;
const abortSignal =
hasAbortController ? abortController!.signal : undefined;
let resolvePending!: () => void;
let rejectPending!: (e: Error) => void;
const pendingPromise = new Promise((res, rej) => {
resolvePending = res;
rejectPending = rej;
});
const promise = task(key, {signal: abortSignal});
// The state is immediately 'pending', since the function has been
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean that even if the promise can't be run with the data available it will take always take two renders to get to initial? One pass for pending and then a second (assuming throw new InitialStateError()) to trigger initial. If so, that seems confusing to end users.

This may be conflating the responsibilities of lit-html and a component system (i.e. LitElement) that relies on it, but I would expect more mainstream usage of this to rely on a key that powered the promise in someway so an initial call where key === undefinedorkey === ''would provide theinitial` render template in a single render pass.

In this case, would you expect there to be additional wrapping of or an alternative to this directive to achieve that functionality? or, would it make sense to include that by default?

// executed, but if the function throws an InitialStateError to
// indicate that it couldn't even start processing, then we will set
// the state to 'initial'.
const runState: AsyncRunState = {
key,
promise,
state: 'pending',
abortController,
resolvePending,
rejectPending,
};
runs.set(part, runState);

Promise.resolve(promise).then(
(value: unknown) => {
runState.state = 'success';
const currentRunState = runs.get(part);
runState.resolvePending();
if (currentRunState !== runState) {
return;
}
part.setValue(success(value));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure the template result for the current success/pending/initial/failure state is re-evaluated (with e.g. the last resolved/error'ed value) for each call to the directive, not just on dependency invalidation, to avoid a footgun where other props in the closure are used in the TR but do not update on change.

We should also be clear in the docs that the dependencies (née key) are what trigger the async function running and thus generating a new promise; however they do not "guard" the template result functions in the way that guard directive would. If the TR functions are expensive, then they would need to nest a guard directive in that value (unless we remove that ability -- in which case there is a slight capability hole there).

part.commit();
},
(error: Error) => {
const currentRunState = runs.get(part);
runState.rejectPending(new Error());
if (currentRunState !== runState) {
return;
}
if (error instanceof InitialStateError &&
typeof initial === 'function') {
runState!.state = 'initial';
part.setValue(initial());
part.commit();
} else {
runState!.state = 'failure';
if (typeof failure === 'function') {
// render success callback
part.setValue(failure(error));
part.commit();
}
}
});

(async () => {
// Wait a microtask for the initial render of the Part to complete
await 0;
const currentRunState = runs.get(part);
if (currentRunState === runState && currentRunState.state === 'pending') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you only dispatch a promise when currentRunState.state === 'pending' an action that immediate throws new InitialStateError() won't be provided the pendingPromise reference to manage the reject that happens on line 108. Should this be updated to:

Suggested change
if (currentRunState === runState && currentRunState.state === 'pending') {
if (currentRunState === runState && (currentRunState.state === 'pending' || currentRunState.state === 'initial')) {

part.startNode.parentNode!.dispatchEvent(
new CustomEvent('pending-state', {
composed: true,
bubbles: true,
detail: {promise: pendingPromise}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider trying to reuse the same pendingPromise when a new promise supersedes a previous unresolved one, and avoid firing the event multiple times?

}));
}
})();
}

// If the promise has not yet resolved, set/update the defaultContent
if ((currentRunState === undefined || currentRunState.state === 'pending') &&

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @justinfagnani : I am experimenting with this a bit since you mentioned it last week. I notice that the pending state is not applied on first key/dependency change after a previous success state - it remains in the success state. Another key change is required for the pending state to be applied.

I could be doing something incorrectly, but I think : on line#93 runs.set(part, runState); is called to set a new pending state, but currentRunState is still the previous success state, so pending() is not called here. Once the directive is run again (ex. second key-stroke in the search demo), currentRunState is initialized with the most recent pending state, and then pending() is called here. It is perhaps not super noticeable when there are many changes (like input event). The absent intermediate pending state was fairly obvious in my case where there aren't multiple key changes quickly in succession.

Copy link
Contributor

@Westbrook Westbrook Jul 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would agree with this. Won't we need to reacquire the values for the current run if we ever want to be able to return to the "pending" state on subsequent uses?

I've been playing with the code shaped this way at https://stackblitz.com/edit/lit-element-run-async-2?file=run-async.ts

Suggested change
if ((currentRunState === undefined || currentRunState.state === 'pending') &&
const runState = runs.get(part);
if ((runState === undefined || runState.state === 'pending') &&

typeof pending === 'function') {
part.setValue(pending());
}
});

/**
* Error thrown by async tasks when the task couldn't be started based on the
* key passed to it.
*/
export class InitialStateError extends Error {}
167 changes: 167 additions & 0 deletions src/test/directives/run-async_test.ts
@@ -0,0 +1,167 @@
/**
* @license
* Copyright (c) 2018 The Polymer Project Authors. All rights reserved.
* This code may only be used under the BSD style license found at
* http://polymer.github.io/LICENSE.txt
* The complete set of authors may be found at
* http://polymer.github.io/AUTHORS.txt
* The complete set of contributors may be found at
* http://polymer.github.io/CONTRIBUTORS.txt
* Code distributed by Google as part of the polymer project is also
* subject to an additional IP rights grant found at
* http://polymer.github.io/PATENTS.txt
*/

import {InitialStateError, runAsync} from '../../directives/run-async.js';
import {render} from '../../lib/render.js';
import {html} from '../../lit-html.js';
import {stripExpressionMarkers} from '../test-utils/strip-markers.js';

const assert = chai.assert;

const hasAbortController = typeof AbortController === 'function';
const testIfHasAbortController = hasAbortController ? test : test.skip;

suite('runAsync', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add tests:

  • Multiple dependencies
  • Add use of props from render closure in template result functions, and ensure they re-evaluate correctly each render
  • Ensure pending-state event is not fired in initial state

let container: HTMLDivElement;

setup(() => {
container = document.createElement('div');
});

const renderTest =
(key: string,
f: (s: string, options: {signal?: AbortSignal}) => Promise<string>) =>
render(
html`${runAsync(key, f, {
success: (s) => html`Success: ${s}`,
pending: () => 'Pending',
initial: () => `Initial`,
failure: (e: Error) => `Error: ${e.message}`
})}`,
container);

test('renders pending then success templates', async () => {
renderTest('test', async (s: string) => s);
assert.equal(stripExpressionMarkers(container.innerHTML), 'Pending');
await 0;
assert.equal(stripExpressionMarkers(container.innerHTML), 'Success: test');
});

test('renders pending then initial templates', async () => {
renderTest('test', async (_: string) => {
throw new InitialStateError();
});
assert.equal(stripExpressionMarkers(container.innerHTML), 'Pending');
await 0;
assert.equal(stripExpressionMarkers(container.innerHTML), 'Initial');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make runAsync wait a microtask after running the async fn to get a shot at rendering initial before pending?

});

test('renders pending then error templates', async () => {
renderTest('test', async (s: string) => {
throw new Error(s);
});
assert.equal(stripExpressionMarkers(container.innerHTML), 'Pending');
await 0;
assert.equal(stripExpressionMarkers(container.innerHTML), 'Error: test');
});

test('fires pending-state with a resolving Promise on success', async () => {
let resolve: () => void;
let pendingPromise: Promise<any>;
container.addEventListener('pending-state', (e: Event) => {
pendingPromise = (e as CustomEvent).detail.promise;
});
renderTest('test', (s: string) => new Promise((r) => resolve = () => r(s)));
assert.equal(stripExpressionMarkers(container.innerHTML), 'Pending');
await 0;
assert.isDefined(pendingPromise!);
resolve!();
// The pending Promise will resolve when the task completes successfully
await pendingPromise!;
assert.equal(stripExpressionMarkers(container.innerHTML), 'Success: test');
});

test('fires pending-state with a rejecting Promise on error', async () => {
let reject: () => void;
let pendingPromise: Promise<any>;
container.addEventListener('pending-state', (e: Event) => {
pendingPromise = (e as CustomEvent).detail.promise;
});
renderTest(
'test',
(s: string) => new Promise((_, r) => reject = () => r(new Error(s))));
assert.equal(stripExpressionMarkers(container.innerHTML), 'Pending');
await 0;
assert.isDefined(pendingPromise!);
reject!();
// The pending Promise will reject when the task completes successfully
try {
await pendingPromise!;
assert.fail();
} catch (e) {
assert.equal(stripExpressionMarkers(container.innerHTML), 'Error: test');
}
});

testIfHasAbortController(
'fires pending-state with a rejecting Promise on abort', async () => {
let pendingPromise: Promise<any>;
container.addEventListener('pending-state', (e: Event) => {
pendingPromise = (e as CustomEvent).detail.promise;
});
renderTest('test', () => new Promise(() => {}));
assert.equal(stripExpressionMarkers(container.innerHTML), 'Pending');
await 0;
assert.isDefined(pendingPromise!);

renderTest('test2', async (s: string) => s);
// The pending Promise will reject when the task completes successfully
try {
await pendingPromise!;
assert.fail();
} catch (e) {
assert.equal(
stripExpressionMarkers(container.innerHTML), 'Success: test2');
}
});

testIfHasAbortController('aborts tasks when key changes', async () => {
let resolve1: () => void;
let aborted = false;
let resolve2: () => void;

// Render with an initial key and a callback that accepts an AbortSignal
renderTest('test', (s: string, {signal}) => {
signal!.addEventListener('abort', () => {
aborted = true;
});
return new Promise((r) => resolve1 = () => r(s));
});
assert.equal(stripExpressionMarkers(container.innerHTML), 'Pending');
await 0;
assert.equal(stripExpressionMarkers(container.innerHTML), 'Pending');

// Render with a new key, which should trigger the previous AbortSignal
renderTest(
'test2', (s: string) => new Promise((r) => resolve2 = () => r(s)));
assert.isTrue(aborted);

// Content is not changed because we're now pending the 2nd key
assert.equal(stripExpressionMarkers(container.innerHTML), 'Pending');
await 0;
assert.equal(stripExpressionMarkers(container.innerHTML), 'Pending');

resolve1!();
// Content is not changed because this result is ignored now
assert.equal(stripExpressionMarkers(container.innerHTML), 'Pending');
await 0;
assert.equal(stripExpressionMarkers(container.innerHTML), 'Pending');

resolve2!();
// Content is updated after a microtask
assert.equal(stripExpressionMarkers(container.innerHTML), 'Pending');
await 0;
assert.equal(stripExpressionMarkers(container.innerHTML), 'Success: test2');
});
});
1 change: 1 addition & 0 deletions test/index.html
Expand Up @@ -20,6 +20,7 @@
<script type="module" src="./directives/guard_test.js"></script>
<script type="module" src="./directives/if-defined_test.js"></script>
<script type="module" src="./directives/repeat_test.js"></script>
<script type="module" src="./directives/run-async_test.js"></script>
<script type="module" src="./directives/until_test.js"></script>
<script type="module" src="./directives/unsafe-html_test.js"></script>
<script type="module" src="./directives/class-map_test.js"></script>
Expand Down