From c700fef09945cc1ddc6eb5f2f5c07fbe599104b6 Mon Sep 17 00:00:00 2001 From: Benny Powers Date: Sun, 15 Sep 2024 09:25:16 +0300 Subject: [PATCH 1/8] feat(core): hasSlotted SSR hints adds ssr-only `ssr-hint-has-slotted="slotA,default,slotB"` attribute to elements which use SlotController --- .../controllers/css-variable-controller.ts | 12 ++- core/pfe-core/controllers/slot-controller.ts | 18 ++-- elements/pf-card/test/pf-card.e2e.ts | 19 +++- tools/pfe-tools/test/playwright/SSRPage.ts | 89 ++++++++++++------- 4 files changed, 95 insertions(+), 43 deletions(-) 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 519adf514d..bb36f4127d 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..c2e035e5f3 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,21 @@ 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'); + expect(fixture.page.locator('pf-card #title')).toHaveAttribute('hidden'); + }); }); diff --git a/tools/pfe-tools/test/playwright/SSRPage.ts b/tools/pfe-tools/test/playwright/SSRPage.ts index 55cd438a32..6f786a2c64 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() { @@ -82,6 +101,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 +134,7 @@ 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.page.$eval(tagName, el => (el as LitElement).updateComplete); } } From 85b983a54470a00a68e8692e21c06e8292319d26 Mon Sep 17 00:00:00 2001 From: Benny Powers Date: Sun, 15 Sep 2024 09:29:42 +0300 Subject: [PATCH 2/8] chore: test workflow --- .github/workflows/tests.yml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 0692846bec..d020aef15b 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 @@ -73,7 +73,7 @@ jobs: - name: Configure node version uses: actions/setup-node@v4 with: - node-version: '20' + node-version-file: .nvmrc cache: npm - name: Install dependencies @@ -100,7 +100,7 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-node@v4 with: - node-version: '20' + node-version-file: .nvmrc cache: npm - run: npm ci --prefer-offline - run: npm run build @@ -110,7 +110,7 @@ jobs: env: HOME: /root - - uses: actions/upload-artifact@v2 + - uses: actions/upload-artifact@v4 if: always() with: name: ${{ env.PLAYWRIGHT_REPORT_DIR }} @@ -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 From 8abb00296e83de6301b882626e04dadba4c01d46 Mon Sep 17 00:00:00 2001 From: Benny Powers Date: Sun, 15 Sep 2024 10:04:08 +0300 Subject: [PATCH 3/8] fix(core): don't load demodir for demoContent ssr tests --- tools/pfe-tools/test/playwright/SSRPage.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/pfe-tools/test/playwright/SSRPage.ts b/tools/pfe-tools/test/playwright/SSRPage.ts index 6f786a2c64..00b2498e06 100644 --- a/tools/pfe-tools/test/playwright/SSRPage.ts +++ b/tools/pfe-tools/test/playwright/SSRPage.ts @@ -91,7 +91,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); } From 4464f24d861f7662693b220335998ad29ae9d6cd Mon Sep 17 00:00:00 2001 From: Benny Powers Date: Sun, 15 Sep 2024 10:34:42 +0300 Subject: [PATCH 4/8] chore: wireit cache --- package.json | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) 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": { From e01a2c4f91bd808c0c4ea374632c0dfc60c0b9f5 Mon Sep 17 00:00:00 2001 From: Benny Powers Date: Sun, 15 Sep 2024 10:34:52 +0300 Subject: [PATCH 5/8] fix(core): ssrpage --- tools/pfe-tools/test/playwright/SSRPage.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tools/pfe-tools/test/playwright/SSRPage.ts b/tools/pfe-tools/test/playwright/SSRPage.ts index 00b2498e06..3d507840a7 100644 --- a/tools/pfe-tools/test/playwright/SSRPage.ts +++ b/tools/pfe-tools/test/playwright/SSRPage.ts @@ -82,6 +82,9 @@ export class SSRPage { javaScriptEnabled: false, })) .newPage(); + if (this.config.demoContent) { + await this.page.goto(`${this.host}test.html`); + } } private async initServer() { @@ -135,6 +138,8 @@ export class SSRPage { } async updateCompleteFor(tagName: string): Promise { + await this.initServer(); + await this.initPage(); await this.page.$eval(tagName, el => (el as LitElement).updateComplete); } } From 4a90b6712bd70806a133f7230cd938750090e767 Mon Sep 17 00:00:00 2001 From: Benny Powers Date: Sun, 15 Sep 2024 10:50:25 +0300 Subject: [PATCH 6/8] chore: e2e workflow --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index d020aef15b..f265413fed 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -130,7 +130,7 @@ jobs: steps: - uses: actions/checkout@v4 - name: Download zipped HTML report - uses: actions/download-artifact@v2 + uses: actions/download-artifact@v4 with: name: ${{ env.PLAYWRIGHT_REPORT_DIR }} path: ${{ env.PLAYWRIGHT_REPORT_DIR }}/ From fb45fb4bd4f6d45d258d943b8645b8414ec29a4c Mon Sep 17 00:00:00 2001 From: Benny Powers Date: Sun, 15 Sep 2024 11:00:40 +0300 Subject: [PATCH 7/8] test(card): fix ssr assert --- elements/pf-card/test/pf-card.e2e.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/elements/pf-card/test/pf-card.e2e.ts b/elements/pf-card/test/pf-card.e2e.ts index c2e035e5f3..4d8c82b54c 100644 --- a/elements/pf-card/test/pf-card.e2e.ts +++ b/elements/pf-card/test/pf-card.e2e.ts @@ -37,6 +37,6 @@ test.describe(tagName, () => { `, }); await fixture.updateCompleteFor('pf-card'); - expect(fixture.page.locator('pf-card #title')).toHaveAttribute('hidden'); + await expect(fixture.page.locator('pf-card #title')).toHaveAttribute('hidden'); }); }); From f470aaa3715ae488adc46db7d0fadc078bfcc2f1 Mon Sep 17 00:00:00 2001 From: Benny Powers Date: Wed, 4 Dec 2024 14:03:51 +0200 Subject: [PATCH 8/8] test(card): ssr test --- elements/pf-card/test/pf-card.e2e.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/elements/pf-card/test/pf-card.e2e.ts b/elements/pf-card/test/pf-card.e2e.ts index 4d8c82b54c..106ae16648 100644 --- a/elements/pf-card/test/pf-card.e2e.ts +++ b/elements/pf-card/test/pf-card.e2e.ts @@ -38,5 +38,6 @@ test.describe(tagName, () => { }); 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'); }); });