diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 7aabf6c219..d620af1c9c 100644 --- a/.github/workflows/tests.yml +++ b/.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 diff --git a/core/pfe-core/controllers/css-variable-controller.ts b/core/pfe-core/controllers/css-variable-controller.ts index ba21f4b9d8..acb5ae32fc 100644 --- a/core/pfe-core/controllers/css-variable-controller.ts +++ b/core/pfe-core/controllers/css-variable-controller.ts @@ -1,10 +1,12 @@ 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) { @@ -12,8 +14,10 @@ export class CssVariableController implements ReactiveController { } 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); + }; } diff --git a/core/pfe-core/controllers/slot-controller.ts b/core/pfe-core/controllers/slot-controller.ts index 6b46cae7b0..a18344b989 100644 --- a/core/pfe-core/controllers/slot-controller.ts +++ b/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); } /** diff --git a/elements/pf-card/test/pf-card.e2e.ts b/elements/pf-card/test/pf-card.e2e.ts index 0d4aac7d7f..106ae16648 100644 --- a/elements/pf-card/test/pf-card.e2e.ts +++ b/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 */ ` + +

Header

+ Body + Footer +
+ `, + }); + 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'); + }); }); diff --git a/package.json b/package.json index 3c99ae30ff..6bf63797e6 100644 --- a/package.json +++ b/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": { diff --git a/tools/pfe-tools/test/playwright/SSRPage.ts b/tools/pfe-tools/test/playwright/SSRPage.ts index 55cd438a32..3d507840a7 100644 --- a/tools/pfe-tools/test/playwright/SSRPage.ts +++ b/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,37 +27,54 @@ 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() { @@ -63,6 +82,9 @@ export class SSRPage { 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 { + await this.initServer(); + await this.initPage(); + await this.page.$eval(tagName, el => (el as LitElement).updateComplete); } }