feat(core): hasSlotted SSR hints (#2845)

* feat(core): hasSlotted SSR hints

adds ssr-only `ssr-hint-has-slotted="slotA,default,slotB"` attribute to
elements which use SlotController

* chore: test workflow

* fix(core): don't load demodir for demoContent ssr tests

* chore: wireit cache

* fix(core): ssrpage

* chore: e2e workflow

* test(card): fix ssr assert

* test(card): ssr test

---------

Co-authored-by: Steven Spriggs <[email protected]>

Author: bennypowers

.github/workflows/tests.yml

@@ -52,7 +52,7 @@ jobs:
       - name: Configure node version
         uses: actions/setup-node@v4
         with:
-          node-version: '20'
+          node-version-file: '.nvmrc'
           cache: npm
 
       - name: Install dependencies
@@ -265,7 +265,7 @@ jobs:
           close-previous: true
           title: "🧪 Tests are failing on main"
           body: "It looks like the build is currently failing on the main branch. See failed [action results](https://github.com/patternfly/patternfly-elements/actions/runs/${{ github.run_id }}) for more details."
-  
+
   build-windows:
     name: Verify that build runs on Windows
     runs-on: windows-latest
@@ -275,9 +275,9 @@ jobs:
 
       # Configures the node version used on GitHub-hosted runners
       - name: Configure node version
-        uses: actions/setup-node@v3
+        uses: actions/setup-node@v4
         with:
-          node-version: 20
+          node-version-file: .nvmrc
           cache: npm
 
       - name: Install dependencies

core/pfe-core/controllers/css-variable-controller.ts

@@ -1,19 +1,23 @@
 import type { ReactiveElement, ReactiveController } from 'lit';
 
 export class CssVariableController implements ReactiveController {
-  style: CSSStyleDeclaration;
+  style?: CSSStyleDeclaration;
 
   constructor(public host: ReactiveElement) {
-    this.style = window.getComputedStyle(host);
+    if (this.host.isConnected) {
+      this.hostConnected();
+    }
   }
 
   private parseProperty(name: string) {
     return name.substring(0, 2) !== '--' ? `--${name}` : name;
   }
 
   getVariable(name: string): string | null {
-    return this.style.getPropertyValue(this.parseProperty(name)).trim() || null;
+    return this.style?.getPropertyValue(this.parseProperty(name)).trim() || null;
   }
 
-  hostConnected?(): void;
+  hostConnected(): void {
+    this.style = window.getComputedStyle(this.host);
+  };
 }

core/pfe-core/controllers/slot-controller.ts

@@ -1,4 +1,4 @@
-import type { ReactiveController, ReactiveElement } from 'lit';
+import { isServer, type ReactiveController, type ReactiveElement } from 'lit';
 
 import { Logger } from './logger.js';
 
@@ -143,11 +143,19 @@ export class SlotController implements ReactiveController {
    * @example this.hasSlotted('header');
    */
   hasSlotted(...names: (string | null | undefined)[]): boolean {
-    const slotNames = Array.from(names, x => x == null ? SlotController.default : x);
-    if (!slotNames.length) {
-      slotNames.push(SlotController.default);
+    if (isServer) {
+      return this.host
+          .getAttribute('ssr-hint-has-slotted')
+          ?.split(',')
+          .map(name => name.trim())
+          .some(name => names.includes(name === 'default' ? null : name)) ?? false;
+    } else {
+      const slotNames = Array.from(names, x => x == null ? SlotController.default : x);
+      if (!slotNames.length) {
+        slotNames.push(SlotController.default);
+      }
+      return slotNames.some(x => this.#nodes.get(x)?.hasContent ?? false);
     }
-    return slotNames.some(x => this.#nodes.get(x)?.hasContent ?? false);
   }
 
   /**

elements/pf-card/test/pf-card.e2e.ts

@@ -1,4 +1,4 @@
-import { test } from '@playwright/test';
+import { test, expect } from '@playwright/test';
 import { PfeDemoPage } from '@patternfly/pfe-tools/test/playwright/PfeDemoPage.js';
 import { SSRPage } from '@patternfly/pfe-tools/test/playwright/SSRPage.js';
 
@@ -22,4 +22,22 @@ test.describe(tagName, () => {
     });
     await fixture.snapshots();
   });
+
+  test('ssr hints', async ({ browser }) => {
+    const fixture = new SSRPage({
+      tagName,
+      browser,
+      importSpecifiers: [`@patternfly/elements/${tagName}/${tagName}.js`],
+      demoContent: /* html */ `
+        <pf-card ssr-hint-has-slotted="header,default,footer">
+          <h2 slot="header">Header</h2>
+          <span>Body</span>
+          <span slot="footer">Footer</span>
+        </pf-card>
+      `,
+    });
+    await fixture.updateCompleteFor('pf-card');
+    await expect(fixture.page.locator('pf-card #title')).toHaveAttribute('hidden');
+    await expect(fixture.page.locator('pf-card #header')).not.toHaveAttribute('hidden');
+  });
 });

package.json

@@ -113,14 +113,13 @@
       "files": [
         "tools/pfe-tools/*.ts",
         "tools/pfe-tools/**/*.ts",
-        "!tools/pfe-tools/*.d.ts",
-        "!tools/pfe-tools/**/*.d.ts"
+        "!tools/**/*.d.ts"
       ],
       "output": [
         "tools/pfe-tools/**/*.js",
         "tools/pfe-tools/**/*.map",
         "tools/pfe-tools/**/*.d.ts",
-        "tools/*.tsbuildinfo"
+        "tools/**/*.tsbuildinfo"
       ]
     },
     "build:core": {
@@ -130,14 +129,14 @@
       ],
       "files": [
         "core/**/*.ts",
-        "!core/**/*.d.ts",
-        "core/tsconfig.json"
+        "core/tsconfig.json",
+        "!core/**/*.d.ts"
       ],
       "output": [
         "core/**/*.js",
         "core/**/*.map",
         "core/**/*.d.ts",
-        "*.tsbuildinfo"
+        "core/**/*.tsbuildinfo"
       ]
     },
     "build:bundle": {

tools/pfe-tools/test/playwright/SSRPage.ts

@@ -9,9 +9,11 @@ import Koa from 'koa';
 import type { Browser, Page } from '@playwright/test';
 import type { Server } from 'node:http';
 import type { AddressInfo } from 'node:net';
+import type { LitElement } from 'lit';
 
-interface SSRDemoConfig {
-  demoDir: URL;
+interface SSRPageConfig {
+  demoDir?: URL;
+  demoContent?: string;
   importSpecifiers: string[];
   tagName: string;
   browser: Browser;
@@ -25,44 +27,64 @@ export class SSRPage {
   private app: Koa;
   private server!: Server;
   private host!: string;
-  private page!: Page;
   private demoPaths!: string[];
 
-  constructor(
-    private config: SSRDemoConfig,
-  ) {
+  public page!: Page;
+
+  constructor(private config: SSRPageConfig) {
     this.app = new Koa();
-    this.app.use(async (ctx, next) => {
+    this.app.use(this.middleware(config));
+  }
+
+  private middleware({ demoContent, demoDir, importSpecifiers }: SSRPageConfig) {
+    return async (ctx: Koa.Context, next: Koa.Next) => {
       if (ctx.method === 'GET') {
-        const origPath = ctx.request.path.replace(/^\//, '');
-        const demoDir = config.demoDir.href;
-        const fileUrl = resolve(demoDir, origPath);
-        if (ctx.request.path.endsWith('.html')) {
+        if (demoContent) {
           try {
-            const content = await readFile(fileURLToPath(fileUrl), 'utf-8');
-            ctx.response.body = await renderGlobal(content, this.config.importSpecifiers);
+            ctx.response.body = await renderGlobal(
+              demoContent,
+              importSpecifiers,
+            );
           } catch (e) {
             ctx.response.status = 500;
             ctx.response.body = (e as Error).stack;
           }
-        } else {
-          try {
-            ctx.response.body = await readFile(fileURLToPath(fileUrl));
-          } catch (e) {
-            ctx.throw(500, e as Error);
+        } else if (demoDir) {
+          const origPath = ctx.request.path.replace(/^\//, '');
+          const { href } = demoDir;
+          const fileUrl = resolve(href, origPath);
+          if (ctx.request.path.endsWith('.html')) {
+            try {
+              const content = await readFile(fileURLToPath(fileUrl), 'utf-8');
+              ctx.response.body = await renderGlobal(content, importSpecifiers);
+            } catch (e) {
+              ctx.response.status = 500;
+              ctx.response.body = (e as Error).stack;
+            }
+          } else {
+            try {
+              ctx.response.body = await readFile(fileURLToPath(fileUrl));
+            } catch (e) {
+              ctx.throw(500, e as Error);
+            }
           }
+        } else {
+          throw new Error('SSRPage must either have a demoDir URL or a demoContent string');
         }
       } else {
         return next();
       }
-    });
+    };
   }
 
   private async initPage() {
     this.page ??= await (await this.config.browser.newContext({
       javaScriptEnabled: false,
     }))
         .newPage();
+    if (this.config.demoContent) {
+      await this.page.goto(`${this.host}test.html`);
+    }
   }
 
   private async initServer() {
@@ -72,7 +94,7 @@ export class SSRPage {
     }
     const { address = 'localhost', port = 0 } = this.server.address() as AddressInfo;
     this.host ??= `http://${address.replace('::', 'localhost')}:${port}/`;
-    this.demoPaths ??= (await readdir(this.config.demoDir))
+    this.demoPaths ??= !this.config.demoDir ? [] : (await readdir(this.config.demoDir))
         .filter(x => x.endsWith('.html'))
         .map(x => new URL(x, this.host).href);
   }
@@ -82,6 +104,22 @@ export class SSRPage {
       !this.server ? rej('no server') : this.server?.close(e => e ? rej(e) : res()));
   }
 
+  /**
+   * Take a visual regression snapshot and save it to disk
+   * @param url url to the demo file
+   */
+  private async snapshot(url: string) {
+    const response = await this.page.goto(url, { waitUntil: 'load' });
+    if (response?.status() === 404) {
+      throw new Error(`Not Found: ${url}`);
+    }
+    expect(response?.status(), await response?.text())
+        .toEqual(200);
+    const snapshot = await this.page.screenshot({ fullPage: true });
+    expect(snapshot, new URL(url).pathname)
+        .toMatchSnapshot(`${this.config.tagName}-${basename(url)}.png`);
+  }
+
   /**
    * Creates visual regression snapshots for each demo in the server's `demoDir`
    */
@@ -99,19 +137,9 @@ export class SSRPage {
     }
   }
 
-  /**
-   * Take a visual regression snapshot and save it to disk
-   * @param url url to the demo file
-   */
-  private async snapshot(url: string) {
-    const response = await this.page.goto(url, { waitUntil: 'load' });
-    if (response?.status() === 404) {
-      throw new Error(`Not Found: ${url}`);
-    }
-    expect(response?.status(), await response?.text())
-        .toEqual(200);
-    const snapshot = await this.page.screenshot({ fullPage: true });
-    expect(snapshot, new URL(url).pathname)
-        .toMatchSnapshot(`${this.config.tagName}-${basename(url)}.png`);
+  async updateCompleteFor(tagName: string): Promise<void> {
+    await this.initServer();
+    await this.initPage();
+    await this.page.$eval(tagName, el => (el as LitElement).updateComplete);
   }
 }