In DEV_MODE, render a warning instead of rendering a template's host in the template. #3199
Conversation
This file contains 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
…in the template.
Most commonly this would happen when rendering `${this}` in a LitElement's template, which has the counterintuitive behavior of removing the element from the DOM, because when rendering the element's template we attach it into its own shadow root, which removes it from the DOM, causing it simply disappear. This is especially problematic with a fast HMR system.
🦋 Changeset detectedLatest commit: e215a65 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📊 Tachometer Benchmark ResultsSummarynop-update
render
update
update-reflect
Resultslit-element-list
render
update
update-reflect
lit-html-kitchen-sink
render
update
nop-update
lit-html-repeat
render
update
lit-html-template-heavy
render
update
reactive-element-list
render
update
update-reflect
|
|
Whoa, this is an unexpected feature to need, but with HMR I get it! But doesn't this potentially apply to any element? What if you wanted to render the id of a child, like Maybe the solution should be to not render any element that already has a parent? I'm also a little surprised that this doesn't just cause an error, since you shouldn't be able to add an element to a descendent as that would cause a cycle in the DOM. |
|
I think it's legit and not that uncommon to reparent a node in general with Lit, but it's almost definitely a mistake to render the host assuming that the host contains the template, as is typical. |
I don't think I've ever seen that. I've definitely seen new, non-parented node be inserted with Lit, but not nodes with a parent already. The main case I could imagine is if the parent is a DocumentFragment like from cloning a template, but since we have the |
|
One use case: render() {
if (this.noChrome) {
return html`${this.contentEl}`;
}
return html`
<header>...</header>
<main>${this.contentEl}</main>
<footer>...</footer>
`;
}Yeah it's just moving the element around inside the same shadow root, but that's legit! Or for a slightly harder to detect example, you could imagine passing contentEl to one of two kinds of rendering element. It's somewhat weird, but there are cases where this is a pretty good design, and if we block it here we're effectively banning it. |
|
otoh, a pattern I have seen not-uncommonly (especially in server-rendered content) is light-DOM-as-data: render() {
const title = this.querySelector('x-title');
return html`<h1>${title.textContent}`</h1>`;
}So how do we keep from destructively updating the DOM when the expression is just I'm wondering if we need an HMR mode with options for these things... |
justinfagnani
approved these changes
Aug 5, 2022
Merged
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.
Most commonly this would happen when rendering
${this}in a LitElement's template, which has the counterintuitive behavior of removing the element from the DOM, because when rendering the element's template we attach it into its own shadow root, which removes it from the DOM, causing it simply disappear. This is especially problematic with a fast HMR system.