Skip to content

Commit a573296

Browse files
authored
♻️ Refactor static site SDK to use functional tab pool + work queue (#150)
## Summary - Add functional tab pool (`pool.js`) using closures for browser tab management - Add task-based processing (`tasks.js`) with dependency injection for testability - Replace page-level concurrency with task-level (page × viewport = task) - Migrate from Vitest to Node native test runner (matching main CLI) - Rename all `.spec.js` to `.test.js` - Remove vitest dependency - Add tests for viewport helpers (`formatViewport`, `setViewport`, `getCommonViewports`) - Add tests for pattern matching edge cases ### Key Architecture Changes **Before:** Page-level concurrency where each page processed viewports sequentially **After:** Task-level concurrency where each (page, viewport) tuple is an independent task **Benefits:** - **True tab control** - Concurrency now means "max tabs" not "max pages" - **Tab reuse** - No create/destroy overhead per screenshot (tabs are pooled) - **Even work distribution** - Tasks processed as capacity allows via work queue - **Functional** - No classes, just functions and closures - **Testable** - Dependency injection enables unit testing without module mocking ### Coverage | Metric | Value | |--------|-------| | Lines | 81.89% | | Branches | 91.81% | | Functions | 73.44% | ## Test plan - [x] All 118 tests pass - [x] Lint passes (biome) - [x] Format passes (biome) - [ ] Manual test with real static site build
1 parent 06432d5 commit a573296

20 files changed

+1781
-3801
lines changed

clients/static-site/package-lock.json

Lines changed: 225 additions & 3123 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

clients/static-site/package.json

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,10 @@
4343
"scripts": {
4444
"build": "npm run clean && npm run compile",
4545
"clean": "rimraf dist",
46-
"compile": "babel src --out-dir dist --ignore '**/*.spec.js'",
46+
"compile": "babel src --out-dir dist --ignore '**/*.test.js'",
4747
"prepublishOnly": "npm run lint && npm run build",
48-
"test": "vitest run",
49-
"test:watch": "vitest",
48+
"test": "node --test --test-reporter=spec $(find tests -name '*.test.js')",
49+
"test:watch": "node --test --test-reporter=spec --watch $(find tests -name '*.test.js')",
5050
"lint": "biome lint src",
5151
"lint:fix": "biome lint --write src",
5252
"format": "biome format --write src",
@@ -76,7 +76,6 @@
7676
"@babel/core": "^7.23.6",
7777
"@babel/preset-env": "^7.23.6",
7878
"@biomejs/biome": "^2.3.8",
79-
"rimraf": "^6.0.1",
80-
"vitest": "^3.2.4"
79+
"rimraf": "^6.0.1"
8180
}
8281
}

clients/static-site/src/browser.js

Lines changed: 2 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
11
/**
22
* Browser management with Puppeteer
3-
* Functions for launching, managing, and closing browsers
3+
* Core functions for launching and managing browsers
44
*/
55

66
import puppeteer from 'puppeteer';
7-
import { setViewport } from './utils/viewport.js';
87

98
/**
109
* Launch a Puppeteer browser instance
@@ -35,15 +34,6 @@ export async function closeBrowser(browser) {
3534
}
3635
}
3736

38-
/**
39-
* Create a new page in the browser
40-
* @param {Object} browser - Browser instance
41-
* @returns {Promise<Object>} Page instance
42-
*/
43-
export async function createPage(browser) {
44-
return await browser.newPage();
45-
}
46-
4737
/**
4838
* Navigate to a URL and wait for the page to load
4939
* @param {Object} page - Puppeteer page instance
@@ -55,7 +45,7 @@ export async function navigateToUrl(page, url, options = {}) {
5545
try {
5646
await page.goto(url, {
5747
waitUntil: 'networkidle2',
58-
timeout: 30000, // 30 second timeout
48+
timeout: 30000,
5949
...options,
6050
});
6151
} catch (error) {
@@ -74,44 +64,3 @@ export async function navigateToUrl(page, url, options = {}) {
7464
}
7565
}
7666
}
77-
78-
/**
79-
* Process a single page - navigate, wait, and prepare for screenshot
80-
* @param {Object} browser - Browser instance
81-
* @param {string} url - Page URL
82-
* @param {Object} viewport - Viewport configuration
83-
* @param {Function|null} beforeScreenshot - Optional hook to run before screenshot
84-
* @returns {Promise<Object>} Page instance ready for screenshot
85-
*/
86-
export async function preparePageForScreenshot(
87-
browser,
88-
url,
89-
viewport,
90-
beforeScreenshot = null
91-
) {
92-
let page = await createPage(browser);
93-
94-
// Set viewport
95-
await setViewport(page, viewport);
96-
97-
// Navigate to page (waits for networkidle2)
98-
await navigateToUrl(page, url);
99-
100-
// Run custom interaction hook if provided
101-
if (beforeScreenshot && typeof beforeScreenshot === 'function') {
102-
await beforeScreenshot(page);
103-
}
104-
105-
return page;
106-
}
107-
108-
/**
109-
* Close a page
110-
* @param {Object} page - Page instance to close
111-
* @returns {Promise<void>}
112-
*/
113-
export async function closePage(page) {
114-
if (page) {
115-
await page.close();
116-
}
117-
}

clients/static-site/src/index.js

Lines changed: 37 additions & 136 deletions
Original file line numberDiff line numberDiff line change
@@ -1,133 +1,22 @@
11
/**
22
* Main entry point for @vizzly-testing/static-site
33
* Functional orchestration of page discovery and screenshot capture
4+
* Uses a tab pool for efficient browser tab management
45
*/
56

6-
import {
7-
closeBrowser,
8-
closePage,
9-
launchBrowser,
10-
preparePageForScreenshot,
11-
} from './browser.js';
12-
import { getPageConfig, loadConfig } from './config.js';
13-
import { discoverPages, generatePageUrl } from './crawler.js';
14-
import { getBeforeScreenshotHook } from './hooks.js';
15-
import { captureAndSendScreenshot } from './screenshot.js';
7+
import { closeBrowser, launchBrowser } from './browser.js';
8+
import { loadConfig } from './config.js';
9+
import { discoverPages } from './crawler.js';
10+
import { createTabPool } from './pool.js';
1611
import { startStaticServer, stopStaticServer } from './server.js';
17-
18-
/**
19-
* Process a single page across all configured viewports
20-
* @param {Object} page - Page object
21-
* @param {Object} browser - Browser instance
22-
* @param {string} baseUrl - Base URL for static site (HTTP server)
23-
* @param {Object} config - Configuration
24-
* @param {Object} context - Plugin context
25-
* @returns {Promise<Object>} Result object with success count and errors
26-
*/
27-
async function processPage(page, browser, baseUrl, config, context) {
28-
let { logger } = context;
29-
let pageConfig = getPageConfig(config, page);
30-
let pageUrl = generatePageUrl(baseUrl, page);
31-
let hook = getBeforeScreenshotHook(page, config);
32-
let errors = [];
33-
34-
// Process each viewport for this page
35-
for (let viewport of pageConfig.viewports) {
36-
let puppeteerPage = null;
37-
38-
try {
39-
puppeteerPage = await preparePageForScreenshot(
40-
browser,
41-
pageUrl,
42-
viewport,
43-
hook
44-
);
45-
await captureAndSendScreenshot(
46-
puppeteerPage,
47-
page,
48-
viewport,
49-
pageConfig.screenshot
50-
);
51-
52-
logger.info(` ✓ ${page.path}@${viewport.name}`);
53-
} catch (error) {
54-
logger.error(` ✗ ${page.path}@${viewport.name}: ${error.message}`);
55-
errors.push({
56-
page: page.path,
57-
viewport: viewport.name,
58-
error: error.message,
59-
});
60-
} finally {
61-
await closePage(puppeteerPage);
62-
}
63-
}
64-
65-
return { errors };
66-
}
67-
68-
/**
69-
* Simple concurrency control - process items with limited parallelism
70-
* @param {Array} items - Items to process
71-
* @param {Function} fn - Async function to process each item
72-
* @param {number} concurrency - Max parallel operations
73-
* @returns {Promise<void>}
74-
*/
75-
async function mapWithConcurrency(items, fn, concurrency) {
76-
let results = [];
77-
let executing = new Set();
78-
79-
for (let item of items) {
80-
let promise = fn(item).then(result => {
81-
executing.delete(promise);
82-
return result;
83-
});
84-
85-
results.push(promise);
86-
executing.add(promise);
87-
88-
if (executing.size >= concurrency) {
89-
await Promise.race(executing);
90-
}
91-
}
92-
93-
await Promise.all(results);
94-
}
95-
96-
/**
97-
* Process all pages with concurrency control
98-
* @param {Array<Object>} pages - Array of page objects
99-
* @param {Object} browser - Browser instance
100-
* @param {string} baseUrl - Base URL for static site (HTTP server)
101-
* @param {Object} config - Configuration
102-
* @param {Object} context - Plugin context
103-
* @returns {Promise<Array>} Array of all errors encountered
104-
*/
105-
async function processPages(pages, browser, baseUrl, config, context) {
106-
let allErrors = [];
107-
108-
await mapWithConcurrency(
109-
pages,
110-
async page => {
111-
let { errors } = await processPage(
112-
page,
113-
browser,
114-
baseUrl,
115-
config,
116-
context
117-
);
118-
allErrors.push(...errors);
119-
},
120-
config.concurrency
121-
);
122-
123-
return allErrors;
124-
}
12+
import { generateTasks, processAllTasks } from './tasks.js';
12513

12614
/**
12715
* Check if TDD mode is available
16+
* @param {Function} [debug] - Optional debug logger
12817
* @returns {Promise<boolean>} True if TDD server is running
12918
*/
130-
async function isTddModeAvailable() {
19+
async function isTddModeAvailable(debug = () => {}) {
13120
let { existsSync, readFileSync } = await import('node:fs');
13221
let { join, parse, dirname } = await import('node:path');
13322

@@ -136,27 +25,34 @@ async function isTddModeAvailable() {
13625
let currentDir = process.cwd();
13726
let root = parse(currentDir).root;
13827

28+
debug(`Searching for TDD server from ${currentDir}`);
29+
13930
while (currentDir !== root) {
14031
let serverJsonPath = join(currentDir, '.vizzly', 'server.json');
14132

14233
if (existsSync(serverJsonPath)) {
34+
debug(`Found server.json at ${serverJsonPath}`);
14335
try {
14436
let serverInfo = JSON.parse(readFileSync(serverJsonPath, 'utf8'));
14537
if (serverInfo.port) {
38+
debug(`Pinging TDD server at port ${serverInfo.port}`);
14639
// Try to ping the server
14740
let response = await fetch(
14841
`http://localhost:${serverInfo.port}/health`
14942
);
43+
debug(`TDD server health check: ${response.ok ? 'OK' : 'FAILED'}`);
15044
return response.ok;
15145
}
152-
} catch {
153-
// Invalid JSON or server not responding
46+
debug('server.json missing port field');
47+
} catch (error) {
48+
debug(`Failed to connect to TDD server: ${error.message}`);
15449
}
15550
}
15651
currentDir = dirname(currentDir);
15752
}
158-
} catch {
159-
// Error checking for TDD mode
53+
debug('No .vizzly/server.json found in parent directories');
54+
} catch (error) {
55+
debug(`Error checking for TDD mode: ${error.message}`);
16056
}
16157

16258
return false;
@@ -173,6 +69,7 @@ function hasApiToken(config) {
17369

17470
/**
17571
* Main run function - orchestrates the entire screenshot capture process
72+
* Uses a tab pool for efficient parallel screenshot capture
17673
* @param {string} buildPath - Path to static site build
17774
* @param {Object} options - CLI options
17875
* @param {Object} context - Plugin context (logger, config, services)
@@ -181,6 +78,7 @@ function hasApiToken(config) {
18178
export async function run(buildPath, options = {}, context = {}) {
18279
let { logger, config: vizzlyConfig, services } = context;
18380
let browser = null;
81+
let pool = null;
18482
let serverInfo = null;
18583
let testRunner = null;
18684
let serverManager = null;
@@ -196,7 +94,8 @@ export async function run(buildPath, options = {}, context = {}) {
19694
let config = await loadConfig(buildPath, options, vizzlyConfig);
19795

19896
// Determine mode: TDD or Run
199-
let isTdd = await isTddModeAvailable();
97+
let debug = logger.debug?.bind(logger) || (() => {});
98+
let isTdd = await isTddModeAvailable(debug);
20099
let hasToken = hasApiToken(vizzlyConfig);
201100

202101
if (isTdd) {
@@ -313,28 +212,27 @@ export async function run(buildPath, options = {}, context = {}) {
313212
return;
314213
}
315214

316-
// Launch browser
215+
// Launch browser and create tab pool
317216
browser = await launchBrowser(config.browser);
217+
pool = createTabPool(browser, config.concurrency);
318218

319-
// Process all pages
320-
let errors = await processPages(
321-
pages,
322-
browser,
323-
serverInfo.url,
324-
config,
325-
context
219+
// Generate all tasks upfront (pages × viewports)
220+
let tasks = generateTasks(pages, serverInfo.url, config);
221+
logger.info(
222+
`📸 Processing ${tasks.length} screenshots (${config.concurrency} concurrent tabs)`
326223
);
327224

225+
// Process all tasks through the tab pool
226+
let errors = await processAllTasks(tasks, pool, config, logger);
227+
328228
// Report summary
329229
if (errors.length > 0) {
330230
logger.warn(`\n⚠️ ${errors.length} screenshot(s) failed:`);
331231
errors.forEach(({ page, viewport, error }) => {
332232
logger.error(` ${page}@${viewport}: ${error}`);
333233
});
334234
} else {
335-
logger.info(
336-
`\n✅ Captured ${pages.length * config.viewports.length} screenshots successfully`
337-
);
235+
logger.info(`\n✅ Captured ${tasks.length} screenshots successfully`);
338236
}
339237

340238
// Finalize build in run mode
@@ -361,7 +259,10 @@ export async function run(buildPath, options = {}, context = {}) {
361259

362260
throw error;
363261
} finally {
364-
// Cleanup
262+
// Cleanup: drain pool first, then close browser
263+
if (pool) {
264+
await pool.drain();
265+
}
365266
if (browser) {
366267
await closeBrowser(browser);
367268
}

0 commit comments

Comments
 (0)