docs(migration-skill): farmrio-storefront wave — gotchas #52-#73, learnings-staging convention - #457
docs(migration-skill): farmrio-storefront wave — gotchas #52-#73, learnings-staging convention#457hugo-ccabral wants to merge 2 commits into
Conversation
…rnings-staging convention Consolidates the farmrio-storefront Fresh→TanStack migration's staged learnings (migration/learnings/, ~69 targets) into this skill's reference docs, per the epic's own T40 target (deco-sites/farmrio-storefront@migration/targets/T40-learnings-to-blocks.md). 22 new numbered gotchas (#52-#73), each with Severity/Symptom/Root cause/Fix/discovery command/empirical farmrio evidence, distributed into their existing topic files: - async-rendering.md: dead 3-arg ctx section loaders (#52, ~28-file sweep, site-wide dead cookie-consent banner), registration-key mismatch (#53), typeof Component === "function" always-false pattern (#54), no response-mutation sink for section loaders (#73) - hydration-fixes.md: DeferredSectionWrapper skeleton remount/CLS (#55, cross-ref #448) + content-refresh-safe Lazy.tsx unwrap pattern, LoadingFallback wrapper-vs-alias (#56), native listener stopPropagation killing React synthetic events (#57), controlled input missing onChange (#58) - vtex-commerce.md: multivariate-flag resolveType hardcoding (#59, blocker), redirect self-loop via case-fold collision (#60, cross-ref #391), raw URL-param auto-injection (#61), missing similars/isSimilarTo (#62), oversized hydration payload from full nested variants (#63), publicUrl protocol mismatch (#64) - css-styling.md: DaisyUI theme slot mismapping (#65), v5 dark-theme auto-bundling (#66), CLS aspect-ratio fallback cropping wide banners (#67) - worker-cloudflare.md: CF Static Assets cache-control bypass (#68), device-segmented cache poisoning via SWR revalidation (#69, novel), registerCacheableSections request-context gap (#70, novel), client-bundle secret leak via invoke dispatch table (#71, security) - storefront-patterns.md: sitemap not auto-wired (#72) Also adds references/migration-learnings-staging.md, proposing the index-per-target-file learnings convention (INDEX.md + one file per target) as the *starting* pattern for the next migration, citing this epic's flat-file cost (280KB/3,875 lines, 60k+ tokens per catch-up read) as the empirical reason it was restructured mid-epic instead. Codemod/audit-rule proposals for packages/blocks-cli and parity-destined findings are in the PR description, not implemented here (proposal is sufficient per the source target's DoD; implementation is a bonus). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
|
||
| **Severity**: BLOCKER — resolves to unenriched/default props on every request, across every migrated site, with zero signal. | ||
|
|
||
| `SectionLoaderFn` in `@decocms/blocks/cms/sectionLoaders.ts` is `(props, req) => ...` — 2 arguments, no `ctx`. Fresh-era loaders carried over as `(props, req, ctx) => ({ ...props, device: ctx.device })` (or reading `ctx.invoke`, `ctx.get`) always resolve `ctx` as `undefined`. `runSingleSectionLoaderImpl` wraps every loader call in a try/catch, so the resulting crash (or silent `ctx.device` → `undefined`) never surfaces anywhere — the section just renders with default/empty values. |
There was a problem hiding this comment.
#52 — description and Fix are outdated
The text says SectionLoaderFn takes 2 args and ctx is always undefined. That does not match the current code:
packages/blocks/src/cms/sectionLoaders.ts:28→ctx?: SectionLoaderContext(3-arg, optional)sectionLoaders.ts:433→withPageContextpassesbuildSectionLoaderContext(req)as the third arg
In practice, 3-arg loaders using ctx.device already work as long as they are registered via registerSectionLoader. The Fix below telling the migrator to "rewrite to ctx-free" will cause them to remove code that was already correct.
Suggestion: add a status note at the top — e.g. "Framework updated: ctx is now properly passed; the Fix below is the safe ctx-free fallback for loaders that prefer to avoid ctx dependency."
|
|
||
| **Discovery command**: | ||
| ```bash | ||
| rg "props\.\w+ ?? .*[Pp]age|Number.isFinite(rawPage|Number.isFinite(props\." packages/apps-vtex/src |
There was a problem hiding this comment.
#61 — broken shell regex (unbalanced parentheses)
The discovery command does not execute — Number.isFinite(rawPage and Number.isFinite(props\. open ( with no closing ) before the |:
# current (does not compile)
rg "props\.\w+ ?? .*[Pp]age|Number.isFinite(rawPage|Number.isFinite(props\." packages/apps-vtex/srcSuggestion: split into two valid rg calls:
rg "props\.\w+ \?\? .*[Pp]age" packages/apps-vtex/src
rg "Number\.isFinite\(props\." packages/apps-vtex/src| ...v.offers, | ||
| offers: v.offers?.offers?.map((o) => ({ | ||
| ...o, | ||
| priceSpecification: o.priceSpecification?.filter((p) => p["@type"] !== "InstallmentPriceSpecification"), |
There was a problem hiding this comment.
#63 — filter matches a type name that never appears in the data
buildPriceSpecification (gotcha #35, line ~110 in this same file) creates installment entries as:
{ "@type": "UnitPriceSpecification", priceComponentType: "https://schema.org/Installment" }But the filter here checks p["@type"] !== "InstallmentPriceSpecification" — a type name that never appears in the data. Every entry passes the filter unchanged, so the payload is never trimmed and the goal of gotcha #63 is silently unmet.
Correct fix:
priceSpecification: o.priceSpecification?.filter(
(p) => p.priceComponentType !== "https://schema.org/Installment"
),| + --color-accent: <value from source mainColors.tertiary>; /* renamed field */ | ||
| ``` | ||
|
|
||
| **Discovery command**: diff every `mainColors`/`complementaryColors` key in the source Theme block against the generated `--color-*` custom properties in `app.css`; flag any generated color with no traceable source origin. |
There was a problem hiding this comment.
#65 — discovery command is prose, not a shell command
This is the only gotcha among the 22 new ones without an executable bash code block — every other gotcha has a rg/grep one-liner. A minimal suggestion:
# generated CSS color vars
grep -oE "\-\-color-[a-z0-9-]+" dist/**/*.css | sort -u
# source theme keys (compare manually against the list above)
jq -r ".. | objects | select(has(\"mainColors\")) | .mainColors | keys[]" \
.deco/blocks/*.json 2>/dev/null | sort -u|
|
||
| --- | ||
|
|
||
| ## #66 DaisyUI v5's default plugin config silently bundles a second dark theme via `prefers-color-scheme` — invisible without forcing dark colorScheme |
There was a problem hiding this comment.
#66 — references DaisyUI v5 while the rest of the file is v4
The file header and existing gotchas (#37, #43, #65) consistently reference DaisyUI v4. This gotcha documents v5 behavior and uses v5 config syntax (@plugin "daisyui" { themes: light --default; }). A migrator working on a v4 project will be confused.
Suggestion: add a one-liner at the top of the section — e.g. "Applies when the migrated site targets DaisyUI v5. See #37 for the v4 equivalent" — to make clear this is not a contradiction with the rest of the file.
|
|
||
| TanStack has no native sitemap renderer — a Fresh/Deco site got this free from the `website`/`commerce` apps. The pieces exist in the installed packages (`@decocms/apps-vtex/utils/sitemap.ts`'s `createVtexSitemapProxy()`, `@decocms/blocks/sdk/sitemap.ts`'s `getCMSSitemapEntries()`/`generateSitemapXml()`) but are never auto-wired into `worker-entry.ts`'s `proxyHandler`. | ||
|
|
||
| **Fix**: wire `createVtexSitemapProxy()` plus a `/sitemap/deco.xml` route as an `extraSitemaps` entry, matching the source site's sitemap index structure. |
There was a problem hiding this comment.
#72 — HIGH/SEO severity gotcha with no code example for the fix
Every other BLOCKER/HIGH gotcha across the files includes a working snippet. The fix text says "wire createVtexSitemapProxy() plus a /sitemap/deco.xml route as an extraSitemaps entry" but shows no code. The JSDoc in packages/apps-vtex/src/utils/sitemap.ts:213 already has the example ready:
import { createVtexSitemapProxy } from "@decocms/apps/vtex/utils/sitemap";
const proxySitemap = createVtexSitemapProxy({
extraSitemaps: ["/sitemap/deco.xml"], // CMS-managed sitemap
});
createDecoWorkerEntry(serverEntry, {
proxyHandler: async (request, url) => {
const sitemap = await proxySitemap(request, url);
if (sitemap) return sitemap;
// ... rest of proxyHandler
return null;
},
});Also: running curl -sI <candidate>/sitemap.xml early in the migration (before manual QA) catches this before it is forgotten.
- #52: ctx is no longer always undefined (framework fixed via issue #305, landed before this PR); reframe Fix as ctx-free alternative, not mandatory - #61: discovery rg command had unbalanced parens and didn't compile; split into two valid rg calls - #63: filter checked "@type" (always "UnitPriceSpecification") instead of priceComponentType, so the trim silently matched nothing; fixed the field - #65: discovery step was prose, not a runnable command; added grep+jq pair - #66: v5-only gotcha wasn't flagged as such in an otherwise v4 file; added an applicability note - #72: fix had no code example unlike every other BLOCKER/HIGH gotcha; added one, correcting the import path typo present in the source JSDoc itself (@decocms/apps/vtex/... -> @decocms/apps-vtex/...) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Summary
Consolidates the farmrio-storefront Fresh→TanStack migration's staged learnings (
migration/learnings/, ~69 completed targets, coverage-verified against the live ledger) into this skill's reference docs, per the epic's own consolidation target (T40).22 new numbered gotchas (#52–#73), each carrying Severity, Symptom, Root cause, Fix (with code), a discovery
rg/grepcommand, and empirical farmrio evidence (file counts, byte deltas, before/after numbers, cross-refs). Distributed into the existing topic files rather than one new dump file, matching this repo's own index-per-topic convention:async-rendering.mdctxsection loaders — 28-file sweep, one was a site-wide dead cookie-consent bannerhydration-fixes.mdDeferredSectionWrapperskeleton remount/CLS (cross-ref #448) + a content-refresh-safeLazy.tsxunwrap patternvtex-commerce.mdcss-styling.mdworker-cloudflare.mdstorefront-patterns.mdsitemap.xmlnot auto-wiredreferences/migration-learnings-staging.md(new file) proposes the index-per-target-file learnings convention as the starting pattern for the next migration's own staging area — citing this epic's own flatLEARNINGS.md(282,249 bytes / 3,875 lines / 103 entries, costing target sessions 60k+ tokens just to search before it was restructured mid-epic on 2026-07-30) as the empirical case for adopting it from day one instead. Linked fromSKILL.md's reference index.Two gotchas (#55, #59) reference upstream PRs/issues already filed from this same migration: decocms/blocks#448 (
DeferredSectionWrapperfix, open) and decocms/blocks#391 (URL param coercion, referenced by #61).Codemod / audit-rule proposals for
packages/blocks-cliProposal only (per this migration's own DoD, implementation is a bonus, not required) — each was hit ≥3 times independently across the farmrio migration:
(props, req, ctx)loader exports, cross-referenced againstregisterSectionLoaderscalls and each file's real__resolveType(not its file path) inblocks.gen.json. Would have caught all 28 candidate files in gotcha fix(migrate): ts-ignore → ts-expect-error + HTMX detection #52 at migration time.typeof X === "function"guard sweep — flag this pattern on any value that originates from CMS section resolution (gotcha feat: unified cache profile system with SWR + SIE #54); recurred independently in 6+ files across the epic.onChangesweep —checked={...}JSX prop with no siblingonChange(gotcha fix: wrap worker fetch in RequestContext.run() #58); found and fixed independently 3 separate times.LoadingFallbackwrapper-function → literal-alias codemod — rewriteexport function LoadingFallback(props) { return <X {...props}/>; }toexport const LoadingFallback = X;(gotcha feat: Chrome Performance trace comparison script #56).blocks.gen.jsonredirect block wherenormalize(from) === normalize(to)after case-folding (gotcha fix(seo): always emit <meta name="robots"> in CMS page head #60); found 3 live instances across 2 targets once the check existed as an ad-hoc script.width/heightback into CMS content, instead of leaving every un-dimensioned image to hit the CLS-safety-net fallback one page at a time (gotcha feat(otel): auto-instrument Workers via OTEL env vars #67).deco-post-cleanup --strictcomment/string-literal awareness — several existing audit rules (deco-cx-runtime-api,style-string-props,htmx-residue) are literal-substring scans with no AST/JSX awareness, so they false-positive on matches inside comments or inside adangerouslySetInnerHTMLHTML string. Farmrio counts: 7 comment-only false positives on one rule, 4 on another, 4-of-6 on a third.Parity-destined findings
Filed separately against
decocms/parity(issue, cross-linked frommigration/learnings/INDEX.mdin farmrio-storefront) rather than bundled into this PR — covers--fail-onrequiring--ci,--pagesnot scopingflows, abanner-aspect-ratiocheck double-counting infinite-carousel clones, alazy-section-presenceheuristic incompatible with TanStack's transport, and LLM visual-diff hallucination on downscaled full-page screenshots.Test plan
gotchas.mdnumbering (fix: add ./apps/autoconfig to package.json exports #51 was the prior max, confirmed viagrep -rn "^## #\?[0-9]" *.md)migration/learnings/T<ID>.mdfile indeco-sites/farmrio-storefront🤖 Generated with Claude Code
Summary by cubic
Consolidates Farm Rio Fresh→TanStack migration learnings into this skill and adds 22 numbered gotchas (#52–#73). Also documents an index-per-target-file staging convention to keep future migrations fast and cheap.
#3053‑arg loader pitfalls,DeferredSectionWrapperCLS, multivariate resolver gaps, sitemap wiring, cache behavior, and a client-bundle secret leak. Cross‑refs:decocms/blocks#448anddecocms/blocks#391.references/migration-learnings-staging.mdand links it fromSKILL.mdto replace flat learnings with an index-per-target approach.Doc updates since review
@decocms/blocksnow supports an optional 3rdctxarg when registered; positions the fix as a ctx‑free alternative for pre‑#305or unregistered loaders.rgcommands.priceComponentType(not@type), ensuring installment entries are removed.grep+jq) for DaisyUI slot mapping.worker-entry.tswiring example for/sitemap.xmland fixes the import path to@decocms/apps-vtex.Written for commit 919eb12. Summary will update on new commits.