feat(vrt): stabilize date-dependent Calendar and DatePicker stories#36437
feat(vrt): stabilize date-dependent Calendar and DatePicker stories#36437PaulGMardling wants to merge 14 commits into
Conversation
📊 Bundle size report✅ No changes found |
There was a problem hiding this comment.
Pull request overview
This PR aims to reduce PR-time Visual Regression Test (VRT) noise by (1) selectively skipping unaffected stories per suite and (2) improving capture determinism by adding an explicit “render ready” signal and syncing the StoryWright patch metadata so immutable installs pass.
Changes:
- Add a CI selector script and workflow steps to compute suite-specific StoryWright
--excludePatternsbased on affected packages, with a safe fallback to full runs. - Wire the computed exclude patterns into each VR suite’s
test-vrscript via env vars. - Harden v9 VRT capture timing via a Storybook readiness marker and a StoryWright patch that waits for it before screenshots.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
yarn.lock |
Sync StoryWright patch hash/checksum so yarn install --immutable succeeds. |
apps/vr-tests/package.json |
Add optional --excludePatterns injection for the v8 VR suite. |
apps/vr-tests-web-components/package.json |
Add optional --excludePatterns injection for the web-components VR suite. |
apps/vr-tests-react-components/package.json |
Add optional --excludePatterns injection for the v9 VR suite. |
apps/vr-tests-react-components/.storybook/preview.js |
Add a VRT readiness signal and asset-wait logic for improved capture stability. |
.yarn/patches/storywright-npm-0.0.27-storybook7.14-bfcc4eb382.patch |
Patch StoryWright to wait for __fluentVrtReady__ before screenshot timing logic. |
.github/workflows/pr-vrt.yml |
Run the selector per suite and export env vars before nx affected -t test-vr. |
.github/scripts/build-vr-storywright-excludes.js |
New script that maps changed packages to story titles and emits StoryWright exclude regexes. |
|
Pull request demo site: URL |
…lure Adds global.d.ts in .storybook/ to augment the Window interface with __fluentVrtReady__. This fixes TS2339 surfaced by vr-tests-react-components:type-check — preview.js uses // @ts-check and the tsconfig has checkJs: true, so the untyped window property assignment was a compile error.
Convert preview.js to use CommonJS require and add proper JSDoc type annotations for all parameters and exports. This resolves all TS errors: - TS1259: React module import (use require instead of import) - TS7006/TS7031: Implicit any type errors on function parameters The file uses @ts-check with checkJs: true in the tsconfig, so JSDoc annotations are necessary instead of TypeScript syntax.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…-platform compatibility
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 10 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
apps/vr-tests-react-components/.storybook/preview.js:27
- The PR description says V9 readiness hardening waits for "fonts/images/frame readiness" before capture, but
waitForVrtAssets()currently only waits for fonts (best-effort) + two animation frames. Either add an image readiness check (best-effort + short timeout) or update the PR description to avoid overstating the behavior.
// Keep readiness checks lightweight to avoid inflating total VRT runtime.
const timeout = new Promise(resolve => setTimeout(resolve, 300));
try {
const fontsReady = document.fonts?.ready;
Avoid shell interpolation when reading changed files, preserve StoryWright arguments without a separator, and include rendered images in the bounded VRT readiness check.
Hotell
left a comment
There was a problem hiding this comment.
General feedback: this adds too much plumbing for uncertain gains.
-
We're not addressing the non-deterministic UI called out in the origin issue (Date.now(), Math.random(), missing value/today props). That was the primary root cause, and it's untouched here — so the noise it produces remains.
-
The timing mechanism (StoryWright patching + manual timers) feels brittle. SW already ships wait APIs that use Playwright under the hood; we should lean on those rather than a global timed heuristic in a vendored patch. If those genuinely don't cover the case, let's experiment with it as a follow-up rather than bundling it here.
-
The affected chain is managed imperatively (regex matchers + alias tables + title token-matching), which is fragile and risks silently skipping stories that should run. The direction we agreed some time ago was collocating VRT stories under the package layer (
<package-name>/visual-regression) so nx affected scopes them natively — verbose, but it does exactly what we need. We can also lean on our workspace plugin to construct these sub-projects dynamically, without touching the file system.
Very fair feedback. |
Pin CalendarCompat and DatePickerCompat dates while removing the selective StoryWright execution and global readiness plumbing.
Previous Behavior
Calendar and DatePicker VRT stories relied on the current date when
todayandvaluewere omitted, causing screenshots to change over time.New Behavior
These stories now use a fixed reference date for deterministic screenshots.
This is the first targeted fix from #35414. Follow-up work will investigate:
CalloutandPositioningpixel drift.