Skip to content

Commit d621778

Browse files
committed
♻️ Refactor static site SDK to use functional tab pool + work queue
- Add functional tab pool (pool.js) using closures for tab management - Add task-based processing (tasks.js) with dependency injection - Replace page-level concurrency with task-level (page×viewport) - Migrate from Vitest to Node native test runner - Rename all .spec.js to .test.js - Remove vitest dependency - Add tests for viewport helpers and pattern edge cases Key improvements: - True tab control: concurrency now means "max tabs" not "max pages" - Tab reuse: no create/destroy overhead per screenshot - Even work distribution: tasks processed as capacity allows - Functional: no classes, just functions and closures Coverage: 81.89% lines, 91.81% branches, 73.44% functions
1 parent 06432d5 commit d621778

19 files changed

+1662
-3720
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: 22 additions & 130 deletions
Original file line numberDiff line numberDiff line change
@@ -1,127 +1,15 @@
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
@@ -173,6 +61,7 @@ function hasApiToken(config) {
17361

17462
/**
17563
* Main run function - orchestrates the entire screenshot capture process
64+
* Uses a tab pool for efficient parallel screenshot capture
17665
* @param {string} buildPath - Path to static site build
17766
* @param {Object} options - CLI options
17867
* @param {Object} context - Plugin context (logger, config, services)
@@ -181,6 +70,7 @@ function hasApiToken(config) {
18170
export async function run(buildPath, options = {}, context = {}) {
18271
let { logger, config: vizzlyConfig, services } = context;
18372
let browser = null;
73+
let pool = null;
18474
let serverInfo = null;
18575
let testRunner = null;
18676
let serverManager = null;
@@ -313,28 +203,27 @@ export async function run(buildPath, options = {}, context = {}) {
313203
return;
314204
}
315205

316-
// Launch browser
206+
// Launch browser and create tab pool
317207
browser = await launchBrowser(config.browser);
208+
pool = createTabPool(browser, config.concurrency);
318209

319-
// Process all pages
320-
let errors = await processPages(
321-
pages,
322-
browser,
323-
serverInfo.url,
324-
config,
325-
context
210+
// Generate all tasks upfront (pages × viewports)
211+
let tasks = generateTasks(pages, serverInfo.url, config);
212+
logger.info(
213+
`📸 Processing ${tasks.length} screenshots (${config.concurrency} concurrent tabs)`
326214
);
327215

216+
// Process all tasks through the tab pool
217+
let errors = await processAllTasks(tasks, pool, config, logger);
218+
328219
// Report summary
329220
if (errors.length > 0) {
330221
logger.warn(`\n⚠️ ${errors.length} screenshot(s) failed:`);
331222
errors.forEach(({ page, viewport, error }) => {
332223
logger.error(` ${page}@${viewport}: ${error}`);
333224
});
334225
} else {
335-
logger.info(
336-
`\n✅ Captured ${pages.length * config.viewports.length} screenshots successfully`
337-
);
226+
logger.info(`\n✅ Captured ${tasks.length} screenshots successfully`);
338227
}
339228

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

362251
throw error;
363252
} finally {
364-
// Cleanup
253+
// Cleanup: drain pool first, then close browser
254+
if (pool) {
255+
await pool.drain();
256+
}
365257
if (browser) {
366258
await closeBrowser(browser);
367259
}

0 commit comments

Comments
 (0)