Skip to content

Commit 3bc3129

Browse files
committed
♻️ split Storybook client wiring
Group the Storybook integration, shared plugin loading, and adjacent client package refs into one review slice.
1 parent 3f22a2a commit 3bc3129

24 files changed

Lines changed: 522 additions & 182 deletions

clients/ember/bin/vizzly-testem-launcher.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,10 @@ async function main() {
103103

104104
// 2. Determine failOnDiff: env var > server.json > default (false)
105105
let failOnDiff = false;
106-
if (process.env.VIZZLY_FAIL_ON_DIFF === 'true' || process.env.VIZZLY_FAIL_ON_DIFF === '1') {
106+
if (
107+
process.env.VIZZLY_FAIL_ON_DIFF === 'true' ||
108+
process.env.VIZZLY_FAIL_ON_DIFF === '1'
109+
) {
107110
failOnDiff = true;
108111
} else {
109112
let serverInfo = getServerInfo();

clients/ember/src/launcher/screenshot-server.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,9 @@ function httpPost(url, data) {
181181

182182
if (res.statusCode >= 400) {
183183
reject(
184-
new Error(`Vizzly server error: ${res.statusCode} - ${responseBody}`)
184+
new Error(
185+
`Vizzly server error: ${res.statusCode} - ${responseBody}`
186+
)
185187
);
186188
return;
187189
}

clients/ember/src/test-support/index.js

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -161,8 +161,10 @@ function prepareTestingContainer(width = 1280, height = 720, fullPage = false) {
161161
* @returns {boolean}
162162
*/
163163
function shouldFailOnDiff() {
164-
return window.__VIZZLY_FAIL_ON_DIFF__ === true ||
165-
window.__VIZZLY_FAIL_ON_DIFF__ === 'true';
164+
return (
165+
window.__VIZZLY_FAIL_ON_DIFF__ === true ||
166+
window.__VIZZLY_FAIL_ON_DIFF__ === 'true'
167+
);
166168
}
167169

168170
/**
@@ -285,7 +287,9 @@ export async function vizzlyScreenshot(name, options = {}) {
285287
// Check if this is a "no server" error - gracefully skip instead of failing
286288
// This allows tests to pass when Vizzly isn't running (like Percy behavior)
287289
if (errorText.includes('No Vizzly server found')) {
288-
console.warn('[vizzly] Vizzly server not running. Skipping visual screenshot.');
290+
console.warn(
291+
'[vizzly] Vizzly server not running. Skipping visual screenshot.'
292+
);
289293
return { status: 'skipped', reason: 'no-server' };
290294
}
291295

@@ -302,12 +306,14 @@ export async function vizzlyScreenshot(name, options = {}) {
302306
if (shouldFail) {
303307
throw new VizzlyDiffError(
304308
`Visual difference detected for '${name}' (${result.diffPercentage?.toFixed(2)}% diff). ` +
305-
`View diff in Vizzly dashboard.`
309+
`View diff in Vizzly dashboard.`
306310
);
307311
}
308312

309313
// Log warning but don't fail
310-
console.warn(`[vizzly] Visual difference detected for '${name}'. View diff in Vizzly dashboard.`);
314+
console.warn(
315+
`[vizzly] Visual difference detected for '${name}'. View diff in Vizzly dashboard.`
316+
);
311317
}
312318

313319
return result;
@@ -320,7 +326,9 @@ export async function vizzlyScreenshot(name, options = {}) {
320326
// Log connection errors only once to avoid spam
321327
if (!hasLoggedConnectionError) {
322328
hasLoggedConnectionError = true;
323-
console.warn(`[vizzly] Screenshots skipped - server not available. Run 'vizzly tdd start' to enable.`);
329+
console.warn(
330+
`[vizzly] Screenshots skipped - server not available. Run 'vizzly tdd start' to enable.`
331+
);
324332
}
325333
return { status: 'skipped', reason: 'error', error: error.message };
326334
} finally {

clients/ember/tests/unit/screenshot-server.test.js

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,10 @@ describe('screenshot-server', () => {
231231
// (unless there's a server.json in a parent directory)
232232
if (info !== null) {
233233
assert.ok(typeof info.url === 'string', 'should have url');
234-
assert.ok(typeof info.failOnDiff === 'boolean', 'should have failOnDiff');
234+
assert.ok(
235+
typeof info.failOnDiff === 'boolean',
236+
'should have failOnDiff'
237+
);
235238
}
236239
} finally {
237240
process.chdir(originalCwd);
@@ -260,8 +263,16 @@ describe('screenshot-server', () => {
260263
let info = getServerInfo();
261264

262265
assert.ok(info !== null, 'should find server.json');
263-
assert.strictEqual(info.url, 'http://localhost:47392', 'should have correct url');
264-
assert.strictEqual(info.failOnDiff, true, 'should read failOnDiff as true');
266+
assert.strictEqual(
267+
info.url,
268+
'http://localhost:47392',
269+
'should have correct url'
270+
);
271+
assert.strictEqual(
272+
info.failOnDiff,
273+
true,
274+
'should read failOnDiff as true'
275+
);
265276
} finally {
266277
process.chdir(originalCwd);
267278
}
@@ -287,8 +298,16 @@ describe('screenshot-server', () => {
287298
let info = getServerInfo();
288299

289300
assert.ok(info !== null, 'should find server.json');
290-
assert.strictEqual(info.url, 'http://localhost:47393', 'should have correct url');
291-
assert.strictEqual(info.failOnDiff, false, 'should default failOnDiff to false');
301+
assert.strictEqual(
302+
info.url,
303+
'http://localhost:47393',
304+
'should have correct url'
305+
);
306+
assert.strictEqual(
307+
info.failOnDiff,
308+
false,
309+
'should default failOnDiff to false'
310+
);
292311
} finally {
293312
process.chdir(originalCwd);
294313
}

clients/ember/tests/unit/testem-config.test.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,10 @@ describe('testem-config', () => {
155155

156156
configure({}, {});
157157

158-
assert.ok(!existsSync(configPath), 'should not write empty playwright.json');
158+
assert.ok(
159+
!existsSync(configPath),
160+
'should not write empty playwright.json'
161+
);
159162
});
160163

161164
it('handles lowercase browser names', () => {

clients/storybook/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ await run('./storybook-static', {
5454
viewports: 'mobile:375x667,desktop:1920x1080',
5555
concurrency: 3,
5656
}, {
57-
logger: console,
57+
output: console,
5858
});
5959
```
6060

clients/storybook/src/crawler.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,9 @@ export function filterStories(stories, config) {
129129
let storyConfig = extractStoryConfig(story);
130130
if (storyConfig?.skip) {
131131
if (verbose) {
132-
console.error(` [filter] Skipping ${story.id} (parameters.vizzly.skip)`);
132+
console.error(
133+
` [filter] Skipping ${story.id} (parameters.vizzly.skip)`
134+
);
133135
}
134136
return false;
135137
}

clients/storybook/src/index.js

Lines changed: 31 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
* Uses a tab pool for efficient browser tab management
55
*/
66

7+
import { existsSync, readFileSync } from 'node:fs';
8+
import { dirname, join, parse } from 'node:path';
79
import { closeBrowser, launchBrowser } from './browser.js';
810
import { loadConfig } from './config.js';
911
import { discoverStories } from './crawler.js';
@@ -16,9 +18,6 @@ import { generateTasks, processAllTasks } from './tasks.js';
1618
* @returns {Promise<boolean>} True if TDD server is running
1719
*/
1820
async function isTddModeAvailable() {
19-
let { existsSync, readFileSync } = await import('node:fs');
20-
let { join, parse, dirname } = await import('node:path');
21-
2221
try {
2322
// Look for .vizzly/server.json
2423
let currentDir = process.cwd();
@@ -64,11 +63,17 @@ function hasApiToken(config) {
6463
* Uses a tab pool for efficient parallel screenshot capture
6564
* @param {string} storybookPath - Path to static Storybook build
6665
* @param {Object} options - CLI options
67-
* @param {Object} context - Plugin context (logger, config, services)
66+
* @param {Object} context - Plugin context (output, config, services)
6867
* @returns {Promise<void>}
6968
*/
7069
export async function run(storybookPath, options = {}, context = {}) {
71-
let { logger, config: vizzlyConfig, services } = context;
70+
let {
71+
output: providedOutput,
72+
logger: legacyOutput,
73+
config: vizzlyConfig,
74+
services,
75+
} = context;
76+
let output = providedOutput || legacyOutput;
7277
let browser = null;
7378
let pool = null;
7479
let serverInfo = null;
@@ -77,8 +82,10 @@ export async function run(storybookPath, options = {}, context = {}) {
7782
let buildId = null;
7883
let startTime = null;
7984

80-
if (!logger) {
81-
throw new Error('Logger is required but was not provided in context');
85+
if (!output) {
86+
throw new Error(
87+
'Output utilities are required but were not provided in context'
88+
);
8289
}
8390

8491
try {
@@ -90,9 +97,9 @@ export async function run(storybookPath, options = {}, context = {}) {
9097
let hasToken = hasApiToken(vizzlyConfig);
9198

9299
if (isTdd) {
93-
logger.info('📍 TDD mode: Using local server');
100+
output.info('📍 TDD mode: Using local server');
94101
} else if (hasToken) {
95-
logger.info('☁️ Run mode: Uploading to cloud');
102+
output.info('☁️ Run mode: Uploading to cloud');
96103
}
97104

98105
let buildUrl = null;
@@ -108,7 +115,7 @@ export async function run(storybookPath, options = {}, context = {}) {
108115
testRunner.once('build-created', buildInfo => {
109116
if (buildInfo.url) {
110117
buildUrl = buildInfo.url;
111-
logger.info(`🔗 ${buildInfo.url}`);
118+
output.info(`🔗 ${buildInfo.url}`);
112119
}
113120
});
114121

@@ -125,7 +132,7 @@ export async function run(storybookPath, options = {}, context = {}) {
125132
pullRequestNumber = gitInfo.prNumber;
126133
} else {
127134
// Fallback for older CLI versions - use environment variables
128-
logger.warn(
135+
output.warn(
129136
'⚠️ Upgrade to @vizzly-testing/cli@>=0.25.0 for improved git detection'
130137
);
131138
branch = process.env.VIZZLY_BRANCH || 'main';
@@ -167,29 +174,29 @@ export async function run(storybookPath, options = {}, context = {}) {
167174
process.env.VIZZLY_ENABLED = 'true';
168175
} catch (error) {
169176
// Log the error and continue without cloud mode
170-
logger.error(`Failed to initialize cloud mode: ${error.message}`);
171-
logger.warn('⚠️ Falling back to local-only mode');
172-
logger.info(' Screenshots will not be uploaded to cloud');
177+
output.error(`Failed to initialize cloud mode: ${error.message}`);
178+
output.warn('⚠️ Falling back to local-only mode');
179+
output.info(' Screenshots will not be uploaded to cloud');
173180
testRunner = null;
174181
}
175182
}
176183

177184
if (!isTdd && !hasToken) {
178-
logger.warn('⚠️ No TDD server or API token found');
179-
logger.info(' Run `vizzly tdd start` or set VIZZLY_TOKEN');
185+
output.warn('⚠️ No TDD server or API token found');
186+
output.info(' Run `vizzly tdd start` or set VIZZLY_TOKEN');
180187
}
181188

182189
// Start HTTP server to serve Storybook static files
183190
serverInfo = await startStaticServer(config.storybookPath);
184191

185192
// Discover stories
186193
let stories = await discoverStories(config.storybookPath, config);
187-
logger.info(
194+
output.info(
188195
`📚 Found ${stories.length} stories in ${config.storybookPath}`
189196
);
190197

191198
if (stories.length === 0) {
192-
logger.warn('⚠️ No stories found');
199+
output.warn('⚠️ No stories found');
193200
return;
194201
}
195202

@@ -199,18 +206,18 @@ export async function run(storybookPath, options = {}, context = {}) {
199206

200207
// Generate all tasks upfront (stories × viewports)
201208
let tasks = generateTasks(stories, serverInfo.url, config);
202-
logger.info(
209+
output.info(
203210
`📸 Processing ${tasks.length} screenshots (${config.concurrency} concurrent tabs)`
204211
);
205212

206213
// Process all tasks through the tab pool
207-
let errors = await processAllTasks(tasks, pool, config, logger);
214+
let errors = await processAllTasks(tasks, pool, config, output);
208215

209216
// Report summary
210217
if (errors.length > 0) {
211-
logger.warn(`\n⚠️ ${errors.length} screenshot(s) failed:`);
218+
output.warn(`\n⚠️ ${errors.length} screenshot(s) failed:`);
212219
errors.forEach(({ story, viewport, error }) => {
213-
logger.error(` ${story}@${viewport}: ${error}`);
220+
output.error(` ${story}@${viewport}: ${error}`);
214221
});
215222
}
216223

@@ -220,11 +227,11 @@ export async function run(storybookPath, options = {}, context = {}) {
220227
await testRunner.finalizeBuild(buildId, false, true, executionTime);
221228

222229
if (buildUrl) {
223-
logger.info(`🔗 View results: ${buildUrl}`);
230+
output.info(`🔗 View results: ${buildUrl}`);
224231
}
225232
}
226233
} catch (error) {
227-
logger.error('Failed to process stories:', error.message);
234+
output.error('Failed to process stories:', error.message);
228235

229236
// Mark build as failed if in run mode
230237
if (testRunner && buildId) {

clients/storybook/src/navigation.js

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,9 @@ export async function navigateToStory(page, storyId, baseUrl, options = {}) {
4747
await fullPageNavigation(page, storyId, baseUrl, timeout);
4848

4949
if (verbose) {
50-
console.error(` [nav] ${storyId}: full-page took ${Date.now() - start}ms`);
50+
console.error(
51+
` [nav] ${storyId}: full-page took ${Date.now() - start}ms`
52+
);
5153
}
5254

5355
if (entry) {
@@ -71,7 +73,9 @@ export async function navigateToStory(page, storyId, baseUrl, options = {}) {
7173
await clientSideNavigation(page, storyId, timeout);
7274

7375
if (verbose) {
74-
console.error(` [nav] ${storyId}: client-side took ${Date.now() - start}ms`);
76+
console.error(
77+
` [nav] ${storyId}: client-side took ${Date.now() - start}ms`
78+
);
7579
}
7680
entry.currentStoryId = storyId;
7781
} catch (error) {
@@ -83,7 +87,9 @@ export async function navigateToStory(page, storyId, baseUrl, options = {}) {
8387
await fullPageNavigation(page, storyId, baseUrl, timeout);
8488

8589
if (verbose) {
86-
console.error(` [nav] ${storyId}: fallback full-page took ${Date.now() - start}ms`);
90+
console.error(
91+
` [nav] ${storyId}: fallback full-page took ${Date.now() - start}ms`
92+
);
8793
}
8894
entry.currentStoryId = storyId;
8995
}
@@ -152,10 +158,10 @@ async function clientSideNavigation(page, storyId, timeout) {
152158
// Navigate to the story
153159
preview.channel.emit('setCurrentStory', { storyId: id });
154160

155-
// Timeout fallback - use configured timeout
161+
// Trigger the full-page fallback when Storybook does not emit a render.
156162
timeoutId = setTimeout(() => {
157163
preview.channel.off('storyRendered', handleRendered);
158-
resolve(); // Resolve anyway - story might have rendered
164+
reject(new Error(`Storybook did not render ${id} before timeout`));
159165
}, timeoutMs);
160166
});
161167
},

clients/storybook/src/plugin.js

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,10 @@ export default {
3838
* @param {import('commander').Command} program - Commander program instance
3939
* @param {Object} context - Plugin context
4040
* @param {Object} context.config - Vizzly configuration
41-
* @param {Object} context.logger - Logger instance
41+
* @param {Object} context.output - Output utilities
4242
* @param {Object} context.services - Service container
4343
*/
44-
register(program, { config, logger, services }) {
44+
register(program, { config, output, services }) {
4545
program
4646
.command('storybook <path>')
4747
.description('Capture screenshots from static Storybook build')
@@ -78,14 +78,14 @@ export default {
7878
let mergedOptions = { ...globalOptions, ...options };
7979

8080
await run(path, mergedOptions, {
81-
logger,
81+
output,
8282
config,
8383
services,
8484
});
8585
} catch (error) {
8686
console.error('Failed to run Storybook plugin:', error);
87-
if (logger?.error) {
88-
logger.error('Failed to run Storybook plugin:', error.message);
87+
if (output?.error) {
88+
output.error('Failed to run Storybook plugin:', error);
8989
}
9090
process.exit(1);
9191
}

0 commit comments

Comments
 (0)