-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
test(solid-start): prerendering - sync #5475 to solid #5627
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,8 +3,9 @@ import { | |
| getDummyServerPort, | ||
| getTestServerPort, | ||
| } from '@tanstack/router-e2e-utils' | ||
| import packageJson from './package.json' with { type: 'json' } | ||
| import { isSpaMode } from './tests/utils/isSpaMode' | ||
| import { isPrerender } from './tests/utils/isPrerender' | ||
| import packageJson from './package.json' with { type: 'json' } | ||
|
|
||
| const PORT = await getTestServerPort( | ||
| `${packageJson.name}${isSpaMode ? '_spa' : ''}`, | ||
|
|
@@ -16,15 +17,21 @@ const EXTERNAL_PORT = await getDummyServerPort(packageJson.name) | |
| const baseURL = `http://localhost:${PORT}` | ||
| const spaModeCommand = `pnpm build:spa && pnpm start:spa` | ||
| const ssrModeCommand = `pnpm build && pnpm start` | ||
| const prerenderModeCommand = `pnpm run test:e2e:startDummyServer && pnpm build:prerender && pnpm run test:e2e:stopDummyServer && pnpm start` | ||
|
|
||
| const getCommand = () => { | ||
| if (isSpaMode) return spaModeCommand | ||
| if (isPrerender) return prerenderModeCommand | ||
| return ssrModeCommand | ||
| } | ||
|
Comment on lines
+20
to
+26
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainReview the dummy server lifecycle management for potential issues. The
The Consider adding error handling to ensure cleanup: -const prerenderModeCommand = `pnpm run test:e2e:startDummyServer && pnpm build:prerender && pnpm run test:e2e:stopDummyServer && pnpm start`
+const prerenderModeCommand = `pnpm run test:e2e:startDummyServer && (pnpm build:prerender; pnpm run test:e2e:stopDummyServer) && pnpm start`Or verify that the dummy server scripts include their own timeout/retry logic to handle startup delays. 🏁 Script executed: #!/bin/bash
# View the playwright config file to see the current state
cat e2e/solid-start/basic/playwright.config.tsLength of output: 1965 🏁 Script executed: #!/bin/bash
# Search for dummy server script definitions in package.json or other config files
fd -t f "package.json" | head -5 | xargs -I {} sh -c 'echo "=== {} ===" && grep -A 2 -B 2 "startDummyServer\|stopDummyServer" {} 2>/dev/null || true'Length of output: 254 🏁 Script executed: #!/bin/bash
# Check for any scripts directory or test setup files that might define the dummy server
fd -t f "(startDummyServer|stopDummyServer|dummy)" e2e/Length of output: 0 🏁 Script executed: #!/bin/bash
# Look for any error handling or cleanup logic in e2e test setup
rg -n "trap\|catch\|error\|cleanup" e2e/solid-start/basic/ --type ts --type jsLength of output: 0 🏁 Script executed: #!/bin/bash
# Search for dummy server script definitions in root package.json
cat package.json | grep -A 100 '"scripts"' | head -50Length of output: 2485 🏁 Script executed: #!/bin/bash
# Search for these specific scripts across the entire codebase
rg -n "startDummyServer|stopDummyServer" --type json --type ts --type js -B 1 -A 1Length of output: 1672 🏁 Script executed: #!/bin/bash
# Check if there's a workspace configuration that might define these scripts
fd -t f "pnpm-workspace.yaml|turbo.json|package.json" -x grep -l "startDummyServer\|stopDummyServer" {} \;Length of output: 73 🏁 Script executed: #!/bin/bash
# View the global setup and teardown files to understand their robustness
cat e2e/solid-start/basic/tests/setup/global.setup.tsLength of output: 220 🏁 Script executed: #!/bin/bash
# View teardown file
cat e2e/solid-start/basic/tests/setup/global.teardown.tsLength of output: 221 🏁 Script executed: #!/bin/bash
# Check if there are any existing error handling patterns in the e2e config or setup
grep -r "trap\|error\|catch\|finally" e2e/solid-start/basic/tests/setup/ --include="*.ts" -nLength of output: 0 Address dummy server lifecycle management issues in prerender command. The resource leak and race condition concerns are confirmed valid:
The suggested fix using subshell syntax Verify and apply one of these approaches:
Same issue exists in 🤖 Prompt for AI Agents |
||
| console.log('running in spa mode: ', isSpaMode.toString()) | ||
| console.log('running in prerender mode: ', isPrerender.toString()) | ||
| /** | ||
| * See https://playwright.dev/docs/test-configuration. | ||
| */ | ||
| export default defineConfig({ | ||
| testDir: './tests', | ||
| workers: 1, | ||
|
|
||
| reporter: [['line']], | ||
|
|
||
| globalSetup: './tests/setup/global.setup.ts', | ||
|
|
@@ -36,7 +43,7 @@ export default defineConfig({ | |
| }, | ||
|
|
||
| webServer: { | ||
| command: isSpaMode ? spaModeCommand : ssrModeCommand, | ||
| command: getCommand(), | ||
| url: baseURL, | ||
| reuseExistingServer: !process.env.CI, | ||
| stdout: 'pipe', | ||
|
|
@@ -53,7 +60,9 @@ export default defineConfig({ | |
| projects: [ | ||
| { | ||
| name: 'chromium', | ||
| use: { ...devices['Desktop Chrome'] }, | ||
| use: { | ||
| ...devices['Desktop Chrome'], | ||
| }, | ||
| }, | ||
| ], | ||
| }) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| import { existsSync, readFileSync } from 'node:fs' | ||
| import { join } from 'node:path' | ||
| import { expect } from '@playwright/test' | ||
| import { test } from '@tanstack/router-e2e-utils' | ||
| import { isPrerender } from './utils/isPrerender' | ||
|
|
||
| test.describe('Prerender Static Path Discovery', () => { | ||
| test.skip(!isPrerender, 'Skipping since not in prerender mode') | ||
| test.describe('Build Output Verification', () => { | ||
| test('should automatically discover and prerender static routes', () => { | ||
| // Check that static routes were automatically discovered and prerendered | ||
| const distDir = join(process.cwd(), 'dist', 'client') | ||
|
|
||
| // These static routes should be automatically discovered and prerendered | ||
| expect(existsSync(join(distDir, 'index.html'))).toBe(true) | ||
| expect(existsSync(join(distDir, 'posts/index.html'))).toBe(true) | ||
| expect(existsSync(join(distDir, 'users/index.html'))).toBe(true) | ||
| expect(existsSync(join(distDir, 'deferred/index.html'))).toBe(true) | ||
| expect(existsSync(join(distDir, 'scripts/index.html'))).toBe(true) | ||
| expect(existsSync(join(distDir, 'inline-scripts/index.html'))).toBe(true) | ||
| expect(existsSync(join(distDir, '대한민국/index.html'))).toBe(true) | ||
|
|
||
| // Pathless layouts should NOT be prerendered (they start with _) | ||
| expect(existsSync(join(distDir, '_layout', 'index.html'))).toBe(false) // /_layout | ||
|
|
||
| // API routes should NOT be prerendered | ||
|
|
||
| expect(existsSync(join(distDir, 'api', 'users', 'index.html'))).toBe( | ||
| false, | ||
| ) // /api/users | ||
| }) | ||
| }) | ||
|
|
||
| test.describe('Static Files Verification', () => { | ||
| test('should contain prerendered content in posts.html', () => { | ||
| const distDir = join(process.cwd(), 'dist', 'client') | ||
| expect(existsSync(join(distDir, 'posts/index.html'))).toBe(true) | ||
|
|
||
| // "Select a post." should be in the prerendered HTML | ||
| const html = readFileSync(join(distDir, 'posts/index.html'), 'utf-8') | ||
| expect(html).toContain('Select a post.') | ||
| }) | ||
|
|
||
| test('should contain prerendered content in users.html', () => { | ||
| const distDir = join(process.cwd(), 'dist', 'client') | ||
| expect(existsSync(join(distDir, 'users/index.html'))).toBe(true) | ||
|
|
||
| // "Select a user." should be in the prerendered HTML | ||
| const html = readFileSync(join(distDir, 'users/index.html'), 'utf-8') | ||
| expect(html).toContain('Select a user.') | ||
| }) | ||
| }) | ||
| }) |
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,6 +1,7 @@ | ||||||||||
| import { expect } from '@playwright/test' | ||||||||||
| import { test } from '@tanstack/router-e2e-utils' | ||||||||||
| import { isSpaMode } from '../tests/utils/isSpaMode' | ||||||||||
| import { isSpaMode } from 'tests/utils/isSpaMode' | ||||||||||
| import { isPrerender } from './utils/isPrerender' | ||||||||||
|
Comment on lines
+3
to
+4
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inconsistent import paths for utilities. Line 3 uses Apply this diff to make imports consistent: -import { isSpaMode } from 'tests/utils/isSpaMode'
-import { isPrerender } from './utils/isPrerender'
+import { isSpaMode } from './utils/isSpaMode'
+import { isPrerender } from './utils/isPrerender'Or if tsconfig paths are preferred: -import { isSpaMode } from 'tests/utils/isSpaMode'
-import { isPrerender } from './utils/isPrerender'
+import { isSpaMode } from 'tests/utils/isSpaMode'
+import { isPrerender } from 'tests/utils/isPrerender'📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||
| import type { Response } from '@playwright/test' | ||||||||||
|
|
||||||||||
| function expectRedirect(response: Response | null, endsWith: string) { | ||||||||||
|
|
@@ -26,9 +27,11 @@ test.describe('/search-params/loader-throws-redirect', () => { | |||||||||
| page, | ||||||||||
| }) => { | ||||||||||
| const response = await page.goto('/search-params/loader-throws-redirect') | ||||||||||
| if (!isSpaMode) { | ||||||||||
|
|
||||||||||
| if (!isSpaMode && !isPrerender) { | ||||||||||
| expectRedirect(response, '/search-params/loader-throws-redirect?step=a') | ||||||||||
| } | ||||||||||
|
|
||||||||||
| await expect(page.getByTestId('search-param')).toContainText('a') | ||||||||||
| expect(page.url().endsWith('/search-params/loader-throws-redirect?step=a')) | ||||||||||
| }) | ||||||||||
|
|
@@ -50,7 +53,7 @@ test.describe('/search-params/default', () => { | |||||||||
| page, | ||||||||||
| }) => { | ||||||||||
| const response = await page.goto('/search-params/default') | ||||||||||
| if (!isSpaMode) { | ||||||||||
| if (!isSpaMode && !isPrerender) { | ||||||||||
| expectRedirect(response, '/search-params/default?default=d1') | ||||||||||
| } | ||||||||||
| await expect(page.getByTestId('search-default')).toContainText('d1') | ||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export const isPrerender: boolean = process.env.MODE === 'prerender' |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -321,4 +321,4 @@ export async function writeBundleToDisk({ | |
|
|
||
| await fsp.writeFile(fullPath, content) | ||
| } | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.