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

updateComplete doesn't guarantee that descendant defined LitElements have been rendered #3538

Open
1 task done
dliebner opened this issue Dec 20, 2022 · 12 comments
Open
1 task done

Comments

@dliebner
Copy link

Should this be an RFC?

  • This is not a substantial change

Which package is this a feature request for?

Lit Core (lit / lit-html / lit-element / reactive-element)

Description

How hard would it be to add a lifecycle method or promise that lets us know:

  • when all defined LitElements that are descendants of this LitElement have been rendered and
  • are also in an updateComplete state?

Alternatives and Workarounds

Running into this issue has made me more aware that LitElements are basically async which is why statefulness is more-or-less required. However, standard HTML is rendered and ready (from a code standpoint) after it has been appended to the DOM, and offering this promise would allow interoperability with that modality of working with elements and DOM trees.

@bennypowers
Copy link
Collaborator

@dliebner
Copy link
Author

Utilizing getUpdateComplete, If I wanted a general solution, where I don't need to explicitly name child nodes (some of which may or may not exist based on render state), would I just need to crawl all the leaf LitElement nodes and wrap their updateComplete in a Promise.all?

@bennypowers
Copy link
Collaborator

override getUpdateComplete() {
  const sup = await super.getUpdateComplete();
  const kids = await Promise.all(
    Array.from(this.children, node => node.updateComplete)
      .filter(x => !!x));
  return sup && kids.every(x => !!x);
}

@dliebner
Copy link
Author

dliebner commented Dec 20, 2022

Very interesting... so I was working on the following in order to get all descendants and stop at LitElements:

function getDescendants(parent, customElementClass) {
  const descendants = [];

  // Use a depth-first search (DFS) algorithm to traverse the DOM tree
  const traverse = node => {
    if (node instanceof customElementClass) {
      descendants.push(node);
      return;  // Stop the traversal when a custom element node is encountered
    }

    for (let i = 0; i < node.childNodes.length; i++) {
      traverse(node.childNodes[i]);
    }
  }

  traverse(parent);
  return descendants;
}

Could I do:

override getUpdateComplete() {
  const sup = await super.getUpdateComplete();
  const kids = await Promise.all(
    Array.from(getDescendants(this, LitElement), node => node.updateComplete)
      .filter(x => !!x));
  return sup && kids.every(x => !!x);
}

I don't understand the boolean casting on updateComplete. Why don't you just stop at Array.from?

EDIT: Okay, actually the filter makes sense, it removes the undefineds. Now I'm trying to figure out why every is used in the return...

@bennypowers
Copy link
Collaborator

bennypowers commented Dec 20, 2022

our goal is to return a promise that resolves to

  • true if the host and every ReactiveElement child is updated
  • false if one of those is not yet updated

so let's

  1. first get the host's state, then after,
  2. get the list of children which are ReactiveElements.
    We duck type by checking for the presence of updateComplete. choose a different way to filter this list! We must filter this list so that only a list of updateComplete promises is returned, for the next step. then,
  3. we resolve every promise in the list and return true if they are all truthy, and
!!(await Promise.resolve(undefined))
// false

@dliebner
Copy link
Author

we resolve every promise in the list and return true if they are all truthy

When would an updateComplete not be truthy? Would the promise returned by the function never resolve in this case?

@bennypowers
Copy link
Collaborator

updateComplete is Promise<boolean>

it would not be truthy when another update was triggered during the previous update cycle. See the docs for more info

@dliebner
Copy link
Author

Ah, I see now. Thanks.

@dliebner
Copy link
Author

Updated DFS descendant walker to use duck typing:

function getDuckTypedDescendants(parent, qualifyingPropertyName) {

	const descendants = [];

	// Use a depth-first search (DFS) algorithm to traverse the DOM tree
	const traverse = node => {

		if( node.hasOwnProperty(qualifyingPropertyName) ) {

			descendants.push(node);
			return;  // Stop the traversal when a custom element node is encountered

		}

		for( let i = 0; i < node.childNodes.length; i++ ) {

			traverse(node.childNodes[i]);

		}
		
	};

	traverse(parent);

	return descendants;

}

// ...

override getUpdateComplete() {
  const sup = await super.getUpdateComplete();
  const kids = await Promise.all(
    Array.from(getDuckTypedDescendants(this, 'updateComplete'), node => node.updateComplete)
      .filter(x => !!x));
  return sup && kids.every(x => !!x);
}

@dliebner
Copy link
Author

In the discord, according to @graynorton:

...under many circumstances the best way to be sure an entire tree of Lit elements has rendered is to await an animation frame after updateComplete on the topmost element. Lit does render asynchronously, but since it uses microtasks for scheduling, you can rely on the tree being rendered within a single frame

My response:

That definitely sounds more performant than walking all the descendants and awaiting their updateComplete promises... if there are multiple levels of custom components, will they all render within one animation frame, or does each level require an animation frame?

@dliebner
Copy link
Author

dliebner commented Dec 20, 2022

Just wanted to contribute one last bit of code, optional implementation:

class ExtendedLitElement extends LitElement {

	async ready( requireEntireDomTreeReady = false ) {

		const thisUpdateComplete = await this.updateComplete;

		if( !requireEntireDomTreeReady ) return thisUpdateComplete;

		const descendants = await Promise.all(
			Array.from(
				internal.getDuckTypedDescendants(this, 'updateComplete'),
				node => typeof node.ready === 'function' ? node.ready(true) : node.updateComplete
			).filter(x => !!x)
		);

		return thisUpdateComplete && descendants.every(x => !!x);

	}

}

All your elements would need to extend this ExtendedLitElement class for use, if you're already using a base extended class like this, you could just add the method to it.

@kevinpschaaf
Copy link
Member

kevinpschaaf commented Jan 11, 2023

updateComplete is admittedly a bit of a middling API, as it only resolves once the immediate component has updated, and not the full tree below it that may have been invalidated; we've held off implementing a fully correct updateCompleteIncludingDescendants type of API both due to performance concerns and the fact that it could still only be correct assuming only Lit components exclusively are used, absent a widely-adopted community protocol.

Based on this, we added a bit more nuance over when to use updateComplete vs. alternatives in the docs in lit/lit.dev#1029 (see https://lit.dev/docs/components/lifecycle/#updatecomplete), since depending on the use case, there are other/better alternatives than that. Hope that helps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

No branches or pull requests

3 participants