[labs/ssr] Make FallbackRenderer not emit a shadow root (#3590)

.changeset/wicked-baboons-pull.md

@@ -2,4 +2,4 @@
 'lit-html': patch
 ---
 
-Add more detail to some hydration errors
+Make FallbackRenderer not emit a shadow root

packages/labs/analyzer/src/lib/utils.ts

@@ -15,7 +15,7 @@ import {hasJSDocTag} from './javascript/jsdoc.js';
 import {Privacy} from './model.js';
 
 export const hasModifier = (node: ts.Node, modifier: ts.SyntaxKind) => {
-return node.modifiers?.some((s) => s.kind === modifier) ?? false;
+  return node.modifiers?.some((s) => s.kind === modifier) ?? false;
 };
 
 export const hasExportModifier = (node: ts.Node) => {

packages/labs/ssr/src/lib/element-renderer.ts

@@ -44,15 +44,19 @@ export const getElementRenderer = (
   return new FallbackRenderer(tagName);
 };
 
-export interface ShadowRootOptions {
-  mode: 'open' | 'closed';
-  delegatesFocus?: boolean;
-}
+// TODO (justinfagnani): remove in favor of ShadowRootInit
+/**
+ * @deprecated Use ShadowRootInit instead
+ */
+export type ShadowRootOptions = ShadowRootInit;
 
 /**
  * An object that renders elements of a certain type.
  */
 export abstract class ElementRenderer {
+  // TODO (justinfagnani): We shouldn't assume that ElementRenderer subclasses
+  // create an element instance. Move this to a base class for renderers that
+  // do.
   element?: HTMLElement;
   tagName: string;
 
@@ -73,28 +77,52 @@ export abstract class ElementRenderer {
     return false;
   }
 
+  /**
+   * Called when a custom element is instantiated during a server render.
+   *
+   * An ElementRenderer can actually instantiate the custom element class, or
+   * it could emulate the element in some other way.
+   */
   constructor(tagName: string) {
     this.tagName = tagName;
   }
 
   /**
-   * Should implement server-appropriate implementation of connectedCallback
+   * Called when a custom element is "attached" to the server DOM.
+   *
+   * Because we don't presume a full DOM emulation, this isn't the same as
+   * being connected in a real browser. There may not be an owner document,
+   * parentNode, etc., depending on the DOM emulation.
+   *
+   * If this renderer is creating actual element instances, it may forward
+   * the call to the element's `connectedCallback()`.
+   *
+   * The default impementation is a no-op.
    */
-  abstract connectedCallback(): void;
+  connectedCallback(): void {
+    // do nothing
+  }
 
   /**
-   * Should implement server-appropriate implementation of attributeChangedCallback
+   * Called from `setAttribute()` to emulate the browser's
+   * `attributeChangedCallback` lifecycle hook.
+   *
+   * If this renderer is creating actual element instances, it may forward
+   * the call to the element's `attributeChangedCallback()`.
    */
-  abstract attributeChangedCallback(
-    name: string,
-    old: string | null,
-    value: string | null
-  ): void;
+  attributeChangedCallback(
+    _name: string,
+    _old: string | null,
+    _value: string | null
+  ) {
+    // do nothing
+  }
 
   /**
-   * Handles setting a property.
+   * Handles setting a property on the element.
    *
-   * Default implementation sets the property on the renderer's element instance.
+   * The default implementation sets the property on the renderer's element
+   * instance.
    *
    * @param name Name of the property
    * @param value Value of the property
@@ -125,27 +153,35 @@ export abstract class ElementRenderer {
   }
 
   /**
-   * Override this getter to configure the element's shadow root, if one is
-   * created with `renderShadow`.
+   * The shadow root options to write to the declarative shadow DOM <template>,
+   * if one is created with `renderShadow()`.
    */
-  get shadowRootOptions(): ShadowRootOptions {
+  get shadowRootOptions(): ShadowRootInit {
     return {mode: 'open'};
   }
 
   /**
-   * Render a single element's ShadowRoot children.
+   * Render the element's shadow root children.
+   *
+   * If `renderShadow()` returns undefined, no declarative shadow root is
+   * emitted.
    */
-  abstract renderShadow(_renderInfo: RenderInfo): RenderResult | undefined;
+  renderShadow(_renderInfo: RenderInfo): RenderResult | undefined {
+    return undefined;
+  }
 
   /**
-   * Render an element's light DOM children.
+   * Render the element's light DOM children.
    */
-  abstract renderLight(renderInfo: RenderInfo): RenderResult | undefined;
+  renderLight(_renderInfo: RenderInfo): RenderResult | undefined {
+    return undefined;
+  }
 
   /**
-   * Render an element's attributes.
+   * Render the element's attributes.
    *
-   * Default implementation serializes all attributes on the element instance.
+   * The default implementation serializes all attributes on the element
+   * instance.
    */
   *renderAttributes(): RenderResult {
     if (this.element !== undefined) {
@@ -169,7 +205,7 @@ export abstract class ElementRenderer {
  * An ElementRenderer used as a fallback in the case where a custom element is
  * either unregistered or has no other matching renderer.
  */
-class FallbackRenderer extends ElementRenderer {
+export class FallbackRenderer extends ElementRenderer {
   private readonly _attributes: {[name: string]: string} = {};
 
   override setAttribute(name: string, value: string) {
@@ -185,9 +221,4 @@ class FallbackRenderer extends ElementRenderer {
       }
     }
   }
-
-  connectedCallback() {}
-  attributeChangedCallback() {}
-  *renderLight() {}
-  *renderShadow() {}
 }

packages/labs/ssr/src/lib/lit-element-renderer.ts

@@ -38,7 +38,7 @@ export class LitElementRenderer extends ElementRenderer {
     );
   }
 
-  connectedCallback() {
+  override connectedCallback() {
     // Call LitElement's `willUpdate` method.
     // Note, this method is required not to use DOM APIs.
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
@@ -49,15 +49,15 @@ export class LitElementRenderer extends ElementRenderer {
     (ReactiveElement.prototype as any).update.call(this.element);
   }
 
-  attributeChangedCallback(
+  override attributeChangedCallback(
     name: string,
     _old: string | null,
     value: string | null
   ) {
     attributeToProperty(this.element as LitElement, name, value);
   }
 
-  *renderShadow(renderInfo: RenderInfo): RenderResult {
+  override *renderShadow(renderInfo: RenderInfo): RenderResult {
     // Render styles.
     const styles = (this.element.constructor as typeof LitElement)
       .elementStyles;
@@ -73,7 +73,7 @@ export class LitElementRenderer extends ElementRenderer {
     yield* renderValue((this.element as any).render(), renderInfo);
   }
 
-  *renderLight(renderInfo: RenderInfo): RenderResult {
+  override *renderLight(renderInfo: RenderInfo): RenderResult {
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     const value = (this.element as any)?.renderLight();
     if (value) {

packages/labs/ssr/src/lib/render-value.ts

@@ -719,25 +719,23 @@ function* renderTemplateResult(
             `Internal error: ${op.type} outside of custom element context`
           );
         }
-        if (instance.renderShadow !== undefined) {
-          renderInfo.customElementHostStack.push(instance);
-          const shadowContents = instance.renderShadow(renderInfo);
-          // Only emit a DSR if renderShadow() emitted something (returning
-          // undefined allows effectively no-op rendering the element)
-          if (shadowContents !== undefined) {
-            const {mode = 'open', delegatesFocus} =
-              instance.shadowRootOptions ?? {};
-            // `delegatesFocus` is intentionally allowed to coerce to boolean to
-            // match web platform behavior.
-            const delegatesfocusAttr = delegatesFocus
-              ? ' shadowrootdelegatesfocus'
-              : '';
-            yield `<template shadowroot="${mode}" shadowrootmode="${mode}"${delegatesfocusAttr}>`;
-            yield* shadowContents;
-            yield '</template>';
-          }
-          renderInfo.customElementHostStack.pop();
+        renderInfo.customElementHostStack.push(instance);
+        const shadowContents = instance.renderShadow(renderInfo);
+        // Only emit a DSR if renderShadow() emitted something (returning
+        // undefined allows effectively no-op rendering the element)
+        if (shadowContents !== undefined) {
+          const {mode = 'open', delegatesFocus} =
+            instance.shadowRootOptions ?? {};
+          // `delegatesFocus` is intentionally allowed to coerce to boolean to
+          // match web platform behavior.
+          const delegatesfocusAttr = delegatesFocus
+            ? ' shadowrootdelegatesfocus'
+            : '';
+          yield `<template shadowroot="${mode}" shadowrootmode="${mode}"${delegatesfocusAttr}>`;
+          yield* shadowContents;
+          yield '</template>';
         }
+        renderInfo.customElementHostStack.pop();
         break;
       }
       case 'custom-element-close':

packages/labs/ssr/src/test/lib/render-lit_test.ts

@@ -10,7 +10,7 @@ import {test} from 'uvu';
 // eslint-disable-next-line import/extensions
 import * as assert from 'uvu/assert';
 import {RenderInfo} from '../../index.js';
-
+import {FallbackRenderer} from '../../lib/element-renderer.js';
 import type * as testModule from '../test-files/render-test-module.js';
 import {collectResultSync} from '../../lib/render-result.js';
 
@@ -251,6 +251,25 @@ for (const global of [emptyVmGlobal, shimmedVmGlobal]) {
     );
   });
 
+  test(`Non-SSR'ed custom element`, async () => {
+    const {render, templateWithNotRenderedElement} = await setup();
+
+    const customElementsRendered: Array<string> = [];
+    const result = await render(templateWithNotRenderedElement, {
+      customElementRendered(tagName: string) {
+        customElementsRendered.push(tagName);
+      },
+      elementRenderers: [FallbackRenderer],
+    });
+    // Undefined elements should not emit a declarative shadowroot
+    assert.is(
+      result,
+      `<!--lit-part drPtGZnekSg=--><test-not-rendered></test-not-rendered><!--/lit-part-->`
+    );
+    assert.is(customElementsRendered.length, 1);
+    assert.is(customElementsRendered[0], 'test-not-rendered');
+  });
+
   test('element with property', async () => {
     const {render, elementWithProperty} = await setup();
     const result = await render(elementWithProperty);

packages/labs/ssr/src/test/test-files/render-test-module.ts

@@ -74,6 +74,13 @@ export class TestSimple extends LitElement {
 // prettier-ignore
 export const simpleTemplateWithElement = html`<test-simple></test-simple>`;
 
+// This must be excluded from rendering in the test
+@customElement('test-not-rendered')
+export class NotRendered extends LitElement {}
+
+// prettier-ignore
+export const templateWithNotRenderedElement = html`<test-not-rendered></test-not-rendered>`;
+
 @customElement('test-property')
 export class TestProperty extends LitElement {
   @property() foo?: string;