fix: two defects the checkout example surfaced - #211
Merged
Conversation
Both were found by building the v2.1.0 commerce example against the published package rather than the working tree, which is why they existed at all. 1. Stamping a page structure erased the canvas genre. `canvas_set_genre` writes metadata.provenance.preset. `applyStructure` then recorded its own provenance by REPLACING the object, so declaring `commerce` and then stamping a layout — the order the docs encourage — silently dropped the genre. Nothing errored; the next evaluation just reported genre.active null and flagged all six money figures on the checkout as fabricated. apply_preset has always merged here. applyStructure now spreads the existing provenance the same way, so the genre and the structure coexist. 2. `export` could not capture a design taller than its artboard. Phase 29 slice E taught `screenshot` to do this and left `export` behind, so saving a long design still meant working out the height by hand. The two render paths had drifted because each carried its own copy of the logic; the viewport-expansion step is now one shared helper, and `export` takes the same `fullPage` option. Ignored for PDF, which paginates on its own. Both are pinned by tests that were checked against the unfixed code first: test-set-genre reports preset=undefined and genre null without fix 1, and test-render-capture exports 800x600 instead of 800x1500 without fix 2. Verified: test:fast 68/68, test:full 100/100, discoverability 117/117.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two of the three findings from building the v2.1.0 commerce example. Both exist because that example was built against the published package rather than the working tree. The third — the eyebrow tell counting form labels — is deliberately not here; it changes evaluator behaviour and deserves measurement rather than a guess.
1. Stamping a page structure erased the canvas genre
canvas_set_genrewritesmetadata.provenance.preset.applyStructurethen recorded its own provenance by replacing that object, so declaringcommerceand then stamping a layout dropped the genre entirely.That is the order the docs encourage — declare what the screen is, start from a vetted pattern — and it failed silently. Nothing errored. The next evaluation simply reported
genre.active: nulland flagged all six money figures on the checkout as fabricated data, which cost a full evaluate cycle to notice and diagnose.apply_presethas always merged into the existing provenance here.applyStructurenow spreads it the same way, so a genre and a structure coexist rather than the later write winning.2.
exportcould not capture a design taller than its artboardPhase 29 slice E taught
screenshotto do this and leftexportbehind, so saving a long design still meant working out the right height by hand — which is exactly what I had to do to produce the checkout screenshot for the README.The underlying reason the two drifted is that each carried its own copy of the render setup. The viewport-expansion step is now a single shared helper used by both, so they cannot disagree again, and
exporttakes the samefullPageoption. It is ignored for PDF, which paginates on its own, and when specificnodeIdsare requested.The helper keeps the original explanation of why Puppeteer's own
fullPageis not used here: it returns the full document on a fresh browser, then silently returns viewport-sized output once any non-fullPage capture has run in the same browser — which, with one browser held for the whole session, is the normal case rather than the edge.Both tests were checked against the unfixed code
A regression test that would not have caught the bug is worth nothing, so I reverted the source and ran them:
test-set-genrereportspreset=undefinedand the evaluator's genre asnulltest-render-captureexports800x600where the content is 1500px tallThe genre test asserts the end effect — that the evaluator still sees
commerce— rather than only the metadata shape, since the metadata is a means and the evaluation is the thing that actually broke.Verified
npm run test:fast68/68,npm run test:full100/100, discoverability 117/117.fullPageonexportis documented in the README options table and in GUIDELINES alongside thescreenshotguidance, so the two read as one option rather than two.