diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..ecaf0a7 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,66 @@ +# Changelog + +Notable changes per release. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); +versioning follows [SemVer](https://semver.org/), with the 0.x caveat that +breaking changes are allowed in minors until 1.0. + +Everything below is pre-publication. Nothing has been released to npm yet, so +this file starts at the first release rather than reconstructing the churn that +led to it — `git log` has that, and it carries the reasoning. + +## [Unreleased] — 0.1.0 + +First release. The runtime, the CLI, and the porting skill. + +### Runtime + +- PJAX navigation over a single configurable container (`#MainContent` by + default), with link interception, history, scroll restoration, and named + transitions. +- Component registry — `setupGlobal()` once, `init(root)` on every load and + swap, optional `destroy(root)`. +- Hooks: `onBeforeNav`, `onBeforeLeave`, `onAfterSwap`, `onAfterInit`, + `onFirstLoad`, `onNavError`. Async handlers are awaited. +- Events on `document`: `pjax:before-nav`, `pjax:content-swap`, + `pjax:islands-revalidated`, and `cart:mutated` as the cart contract. +- Prefetch with per-template TTLs, soft/hard staleness, hover, pointer-down, + focus and viewport warming, and a concurrency cap of two so speculative work + never starves a real request. +- Islands — `[data-island][data-section-id]` regions revalidated through the + Section Rendering API after a cached navigation. +- Head sync for title, meta, `body[data-template]`, scripts and stylesheets. +- Analytics bridges: `shopify`, `customEvents`, `standardEvents`, `trekkie`, + `ga4`, `dataLayer`. Standard Web Pixel events are **not** reachable on a soft + navigation — this is a platform constraint, measured rather than assumed, and + the README's "Analytics & tracking" section says what does and does not work. +- Standard storefront events — `shopify:page:view` is re-dispatched per + navigation, which is the sanctioned way app code learns a soft navigation + happened. +- Accessibility: focus moves to the hash target, then an `[autofocus]` element + in the incoming content, then the container; an `aria-live` region announces + the new title; `prefers-reduced-motion` skips transitions. +- Theme editor: PJAX is off under `designMode`; `shopify:section:load` and + `shopify:section:unload` are handled. +- `appCompat.sectionEvents` and `appCompat.reexecuteExtensionScripts` — + experimental theme-app-extension repairs, both **off by default**. Enabling + both together compounds multiplicatively and warns at boot. + +### CLI + +- `pusha audit` — classifies every script in a theme into buckets A–H and K, + plus the surface buckets J, L, M, P and X. `--json` makes the report + addressable by an agent, narrowable with `--bucket`, `--action` and `--file`. +- `pusha init` — Path A (vendored `pusha.min.js` + snippet) or Path B (npm), + detected from the theme. +- `pusha skill` — installs the porting skill for Claude Code, Cursor or Aider. + +### Known limitations + +- Pixels wired through Shopify admin (Meta, GA4, TikTok, Klaviyo) do not + receive a pageview on a soft navigation. No supported theme-side fix exists + today. See the README. +- Theme app extension code with no re-init hook goes inert after a swap, + regardless of how it is loaded. Apps that listen for `shopify:page:view` or + that ship custom elements recover for free. +- `` and `` are not synced. Route language switches + through a full browser navigation. diff --git a/CLAUDE.md b/CLAUDE.md index d029f21..184cb2c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -118,7 +118,9 @@ This is the command that converts a curious reader into a user. It's also where #### `npx @mimeticthemes/pusha audit` -Runs the audit against the current directory (or path argument). Buckets A–H (per-script) plus the surface buckets: J (analytics), K (portal-to-body custom elements), L (per-request Liquid in the shell), M (persistent-shell stateful UI), P (partials). `--json` emits structured output for tools/agents. The CLI is the canonical audit implementation — there is no longer a parallel bash script. +Runs the audit against the current directory (or path argument). Buckets A–H and K (per-script) plus the surface buckets: J (analytics), L (per-request Liquid in the shell), M (persistent-shell stateful UI), P (partials), R (island candidates), X (theme app extensions). `--json` emits structured output for tools/agents. The CLI is the canonical audit implementation — there is no longer a parallel bash script. + +The authority on this list is `BUCKET_RULES` in `bin/pusha.js`; `skill/SKILL.md` restates it for the porting agent. All three drifted once already — X shipped in the CLI and the skill while this line still stopped at P. #### `npx @mimeticthemes/pusha init` @@ -286,7 +288,28 @@ The Shopify Section Rendering API lets you fetch any section's HTML in isolation - On navigations where prefetchConfig's `soft` TTL has elapsed but `hard` has not — page HTML is reused, islands fetched fresh - Never on uncached navigations — the page was just fetched, everything is current -**Visual contract**: while an island is revalidating, it gets a `.is-revalidating` class that themes can style for a subtle dim/skeleton. The class is removed when the new HTML is in place. +**Visual contract**: while an island is revalidating, it gets a `.is-revalidating` class that themes can style for a subtle dim/skeleton. The class is removed when the revalidation cycle ends — **on every path, including the ones that swap nothing**. It used to come off only because `wrapper.replaceWith()` destroyed the node carrying it, so an island whose `#shopify-section-` wrapper was absent kept the class on a live element forever and a themed dim never lifted. Found in a browser, 2026-09-12; the diagnostic that should have caught it counted keys in the response rather than nodes replaced, so it printed `swapped 1` for a run that swapped nothing. + +**Never send `Accept: application/json` on the sections request.** On a product +URL that header triggers Shopify's product-JSON content negotiation, which beats +the query string: `?sections=` is ignored and the response is `{"product": {…}}` +at status 200 with content-type `application/json`. Every check downstream +passes, the single key names no section, and nothing is swapped. Islands +therefore never worked on a product page — the case they exist for — from the +first release until 2026-09-12, and no test caught it because jsdom fixtures +answer whatever the stub is told to answer. Measured with curl on the same URL: +with the header, the product object; without it, the section HTML keyed by the +requested id. The Section Rendering API keys off `?sections=` and needs no +Accept header. + +**The audit proposes islands; it never marks them.** Bucket R lists stale-prone +regions inside the container (price, availability, inventory, selected variant, +render-time values) and checks conformance of any `data-island` already present. +Candidates are `decide`, never `transform`: revalidating costs a request and +re-runs the section's JS, so whether a region is stale-prone enough to spend +that on is a question about the store. Broken markers — no `data-section-id`, a +marker in the shell, a snippet whose render sites pass no `section:` — are +`transform`, because those are mechanical and each one fails silently. **Exported as `@mimeticthemes/pusha/islands` subpath** for explicit imports. Auto-imported by main entry when prefetch is enabled. @@ -410,7 +433,7 @@ Themes that want fine-grained control populate `cartStatefulRoutes` with the rou PJAX nav has real a11y implications. The runtime handles: -- **Focus restoration**: after DOM swap, focus is moved to either the URL hash target (if any) or to `#MainContent` (or whatever element matches `[data-page-container]`). Without this, keyboard users get lost after every nav. +- **Focus restoration**: after DOM swap, focus is moved to the first of these that exists — the URL hash target, an `[autofocus]` element inside the incoming content, `#MainContent` (or whatever matches `[data-page-container]`). Without this, keyboard users get lost after every nav. The `[autofocus]` step exists because the browser will not run its own autofocus processing on swapped-in content (the document already has a focused element, so it declines and logs a warning), which would otherwise make the attribute silently dead on every soft navigation. The hash wins over `[autofocus]`: it is intent expressed later than the markup. - **Screen reader announcement**: an offscreen `aria-live="polite"` region is updated with the new page title after PJAX swap, so screen readers announce the navigation. Without this, the URL silently changes and assistive tech doesn't know. - **Reduced motion**: when `prefers-reduced-motion: reduce` matches, transitions are skipped (instant swap, no `is-transitioning-*` classes). Themes can override per-transition. @@ -518,14 +541,27 @@ Earlier text here described prefetch as an "opt-in plugin (Barba-style — core small, prefetch is optional)"; that was intent, never implementation, and it is removed rather than restated so nobody designs against it. -**This is a deliberate hold, not an oversight.** Measured 2026-09-11 on a clean -build: 32.2 kB raw / **10.7 kB gzipped** for the UMD bundle — already under -`@barba/core` alone (9.9 kB) before Barba adds the plugins needed to match, and -that is with the analytics bridges, islands and the a11y handling included. -Tree-shaking would save single-digit kB at best, Path A cannot benefit from it at -all (one UMD file by construction), and moving optional subsystems behind dynamic -imports would add a round-trip to the first navigation — the exact latency the -product sells against. +**This is a deliberate hold, not an oversight.** Measured 2026-09-12 on a clean +build: 35.2 kB raw / **11.7 kB gzipped** for the UMD bundle, 27.1 kB / 8.1 kB for +the ESM entry — with the analytics bridges, islands and the a11y handling +included. Tree-shaking would save single-digit kB at best, Path A cannot benefit +from it at all (one UMD file by construction), and moving optional subsystems +behind dynamic imports would add a round-trip to the first navigation — the exact +latency the product sells against. + +⚠ **This paragraph previously claimed 10.7 kB gzipped and "already under +`@barba/core` alone (9.9 kB)".** Both were wrong, and the second inverted the +argument: at 11.7 kB Pusha is *larger* than Barba's core, not smaller. The README +had the honest framing on the same day — comparable feature set means +`@barba/core` + `@barba/prefetch` + `@barba/css` + `@barba/router` at ~12.5 kB, +and that is still without an analytics bridge or Section Rendering API +revalidation. Keep the comparison feature-for-feature; the core-only number +flatters us and does not survive a reader checking it. + +Bundle figures are measured, so they go stale on every merge. Re-measure with +`npm run build` before quoting them, and keep this paragraph and the README's +"Bundle sizes" table in step — they disagreed for a day while both claimed the +same measurement date. **The trigger to revisit is a subsystem growing, not a size threshold.** `app-compat.ts` is ~0.3 kB gzipped today (measured by unwiring it and rebuilding). diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..8407b3e --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,78 @@ +# Contributing + +Pusha is small and opinionated, and it is pre-1.0. The most useful contribution +right now is a theme it breaks on — a real Online Store 2.0 theme, the audit +output, and what went wrong in the browser. + +## Setup + +```sh +npm install +npm test # node's test runner over test/*.test.ts, jsdom fixtures +npm run typecheck +npm run build # vite ESM + UMD, then tsc for the .d.ts files +``` + +Node `^22.13.0 || >=24.0.0`. CI runs both, and it has caught a real difference +between them at least once — if a test passes locally and fails on 22.13, that +is a signal, not noise. + +## The four surfaces must agree + +`pusha init`, `pusha audit`, the skill (`skill/SKILL.md` + `skill/PATTERNS.md`), +and the README describe one contract to four different readers. Three of them +are consumed by an agent or a machine rather than a person, so a stale one does +not read as out of date — it reads as authoritative and wrong. An audit rule +that describes a removed config key teaches the removed key to every agent that +runs it. + +Any change to config keys, defaults, hook or event names and payloads, bucket +rules or their remediation text, install layout, or the `--json` contract +touches all of these: + +1. Source (`src/`) — the behaviour. +2. `bin/pusha.js` — `audit` rules and remediation, `init` writes. +3. `skill/SKILL.md` and `skill/PATTERNS.md` — procedure and patterns. +4. `README.md` — the documented contract, and the tie-breaker when two + surfaces disagree. +5. Tests, including the audit golden files. + +If one genuinely does not apply, say so in the commit message. Silence reads as +an oversight, because usually it is. + +## Tests + +Every bug fix gets a test that fails without the fix. Check that it does — +comment the fix out, watch it go red, put it back. A regression test that never +failed is decoration. + +Assert on the damage, not on the mechanism. A test that asserts "no stray +request reached the wire" keeps working when the internals move; one that +asserts on a private counter breaks for reasons that are not bugs. + +The jsdom fixture in `test/dom.ts` is deliberately thin. jsdom leaves some DOM +APIs undefined rather than warning, so a code path can be silently unreachable +from the suite — `Element.scrollIntoView` hid the whole hash branch of +`focus.ts` until it was stubbed. If a branch looks covered but has never gone +red, check that the API it depends on exists in the fixture. + +## Commits + +Write the reasoning, not the diff. The subject says what changed; the body says +what was true that made it necessary and what you ruled out. `git log` is where +this project keeps its reasoning, so a message that only restates the patch +throws that away. + +## Browser verification + +jsdom coverage is not browser coverage, and the gap is not theoretical: the +abort race, the navigation timeout, and hash-link handling all needed a real +browser to be believed. Anything touching navigation timing, focus, or +transitions should say in the commit message whether it was walked in a browser +or only tested in jsdom. + +## Scope + +Cart is theme code. Analytics is framework code. If you are not sure which side +of that line something falls on, the README's "Cart is theme code" section +explains the test being applied. diff --git a/README.md b/README.md index 8278699..172c213 100644 --- a/README.md +++ b/README.md @@ -157,7 +157,7 @@ npx github:mimetic-themes/pusha init This copies `pusha.min.js` into `assets/`, `pusha.liquid` into `snippets/`, and (with confirmation) inserts `{% render 'pusha' %}` into `layout/theme.liquid` before ``. -Flags: `--dry-run`, `--force`, `--yes` / `-y`. +Flags: `--dry-run`, `--force`, `--yes` / `-y`, `--skip-detect`. ### Path B — bundler @@ -709,7 +709,7 @@ Dispatched on `document`: |---|---|---| | `pjax:before-nav` | `{ url, event }` | Cancelable. `preventDefault()` on the CustomEvent falls through to a full nav. | | `pjax:content-swap` | `{ url, template, cached }` | Fires after `onAfterInit`. | -| `pjax:islands-revalidated` | `{ sectionIds }` | Fires after Section Rendering API revalidation. | +| `pjax:islands-revalidated` | `{ sectionIds, requestedIds }` | Fires after Section Rendering API revalidation. `sectionIds` is what was actually swapped into the page; `requestedIds` is what was asked for. They differ when the server renders a section this document has no `#shopify-section-` wrapper for. | | `cart:mutated` | `{ source, cart?, lastOperation? }` | **The theme dispatches this** after any cart change. Pusha listens and invalidates prefetch — and also dispatches it itself, with `source: 'shopify-standard-events'`, when a standard cart event settles. See [Cart](#cart). | ### Islands (Section Rendering API) @@ -723,7 +723,9 @@ For inventory- and price-sensitive regions inside templates that use long prefet ``` -After a cached navigation, Pusha fetches `?sections=section-id-1,section-id-2`, parses the JSON response, and hot-swaps the section markup. While revalidating, the island gets `.is-revalidating` for a subtle dim/skeleton. +After a cached navigation, Pusha fetches `?sections=section-id-1,section-id-2`, parses the JSON response, and hot-swaps the section markup. While revalidating, the island gets `.is-revalidating` for a subtle dim/skeleton; it is removed when the cycle ends, whether or not anything was swapped. + +The island marker and the `#shopify-section-` wrapper it names have to be in the same document. Shopify returns section HTML keyed by section id, and Pusha replaces `#shopify-section-`, so an island whose `data-section-id` has no matching wrapper is fetched and then has nowhere to go. Run with `debug: true` and the log says `NO TARGET` for exactly that case. ### Cart @@ -906,7 +908,8 @@ Sections written to be re-init-safe under page swaps work for the theme editor a Built in, not configurable: -- **Focus restoration** — focus moves to the URL hash target (if any) or to the swap container after every nav. Without this, keyboard users get stuck on the now-removed link. +- **Focus restoration** — after every nav focus moves to the first of these that exists: the URL hash target, an `[autofocus]` element in the incoming content, the swap container. Without this, keyboard users get stuck on the now-removed link. +- **`[autofocus]` keeps working on a soft navigation** — the browser declines to process it, because swapped-in content arrives while the document already has a focused element. Pusha applies it instead. Put `autofocus` on the search field of a search template and it behaves the same whether the page was loaded or navigated to. An `[autofocus]` that cannot take focus (disabled, hidden, not focusable) falls through to the container. - **Screen-reader announcement** — an offscreen `aria-live="polite"` region is updated with the new page title. - **Reduced motion** — when `prefers-reduced-motion: reduce` matches, transitions are skipped (instant swap, no `is-transitioning-*` classes). @@ -914,12 +917,12 @@ Built in, not configurable: ## Bundle sizes -Measured from a clean `npm run build` on 2026-09-11: +Measured from a clean `npm run build` on 2026-09-12: | File | Raw | Gzipped | |---|---|---| -| `dist/pusha.min.js` (UMD, prod) | 34.7 kB | **11.5 kB** | -| `dist/pusha.esm.js` (ESM, main entry) | 26.9 kB | 8.1 kB | +| `dist/pusha.min.js` (UMD, prod) | 35.3 kB | **11.7 kB** | +| `dist/pusha.esm.js` (ESM, main entry) | 27.2 kB | 8.2 kB | Grown from 9.1 kB gzipped in the first build — the analytics bridges, the standard cart event bridge, and the app-compatibility flags landed since. The UMD bundle is everything — navigation, prefetch cache, @@ -967,18 +970,22 @@ Until Pusha is published, run the CLI straight from this repository with `npx github:mimetic-themes/pusha `. After a Path B install it is on your project's `$PATH` as `pusha`. -`init` flags: `--dry-run`, `--force`, `--yes` / `-y`. +`init` flags: `--dry-run`, `--force`, `--yes` / `-y`, `--skip-detect` (install +even when the directory does not look like a theme). `audit` flags: `--json` for structured output, `--full` to append the whole of `PATTERNS.md` to the report (one self-contained doc an agent can read in a single pass), `--no-whitelist` to disable the false-positive filters and see -every raw finding. Pass a path to audit a different directory. +every raw finding, `--ignore-manifest` to ignore `.pusha/MANIFEST.md` and treat +every finding as unsettled. With `--json`, narrow the report with `--bucket`, +`--action` (`transform` | `decide` | `verify` | `none`) and `--file`. Pass a +path to audit a different directory. `skill` flags: `--print` to dump `SKILL.md` + `PATTERNS.md` to stdout, or `--claude` / `--cursor` / `--aider` to install the skill for that agent. Add `--global` to install into `~/` instead of the project. -The audit classifies every script in the theme by transformation difficulty (buckets A–H), plus the buckets that cover a whole surface rather than a single script: J (analytics — coverage, payload conformance, placement, and raw pixels that bypass Customer Events), K (portal-to-body custom elements needing `data-pusha-cleanup`), L (per-request Liquid frozen in the shell), M (persistent-shell stateful UI), P (`{% partial %}` regions), and X (theme app extensions — which installed app blocks and embeds sit inside the swap container, and which survive it). The `pusha` skill consumes this output to apply the wrappers; agents without the skill can act on the audit's prescriptive "Next steps" block directly. +The audit classifies every script in the theme by transformation difficulty (buckets A–H, plus K for portal-to-body custom elements needing `data-pusha-cleanup`), and then the buckets that cover a whole surface rather than a single script: J (analytics — coverage, payload conformance, placement, and raw pixels that bypass Customer Events), L (per-request Liquid frozen in the shell), M (persistent-shell stateful UI), P (`{% partial %}` regions), R (island candidates — stale-prone regions inside the container, and conformance of any `data-island` markers already there), and X (theme app extensions — which installed app blocks and embeds sit inside the swap container, and which survive it). The `pusha` skill consumes this output to apply the wrappers; agents without the skill can act on the audit's prescriptive "Next steps" block directly. A `create-pusha` scaffolder for new themes is on the roadmap, once Pusha is published. diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 0000000..fa73457 --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,49 @@ +# Security + +## Reporting + +Use **GitHub's private vulnerability reporting** on this repository (Security → +Report a vulnerability). Please do not open a public issue for anything +exploitable. + +Include what you did, what happened, and the theme shape it happened on. A +proof of concept helps; a working exploit against someone else's live store +does not — do not test against storefronts you do not control. + +Expect an acknowledgement within a week. Pusha is maintained by a one-person +studio, so that is a realistic figure rather than an aspirational one. + +## Scope + +Pusha runs in the storefront, in the buyer's browser, with whatever privileges +the theme already has. What is in scope: + +- Parsing fetched HTML into the live document — the swap path, head sync, and + Section Rendering API island responses. +- The origin checks on navigation and prefetch. Pusha refuses cross-origin + redirects rather than parsing another origin's markup as if it were the + shop's; a way around that is a vulnerability. +- `syncHeadScripts` / `syncHeadStyles`, which execute script and style elements + from a fetched document. +- The `appCompat.reexecuteExtensionScripts` flag, which re-executes bundles + from `cdn.shopify.com/extensions/`. It is off by default and its known cost + (unbounded listener accumulation) is documented, not a finding — but a way to + make it execute from another origin is. +- `bin/pusha.js`, which writes into a developer's theme directory. Path + traversal or writing outside the target theme is in scope. + +What is not: + +- Theme code Pusha navigates between. A theme that injects untrusted HTML is + vulnerable whether or not it soft-navigates. +- Shopify's own platform, including Web Pixels, the Section Rendering API, and + Standard storefront events. Report those to Shopify. +- The fact that a soft navigation does not re-run a third-party app's + once-per-document script. That is a documented compatibility limitation with + measurements behind it, not a security issue. + +## Supported versions + +Pre-1.0. Fixes land on the current minor; there are no backports. Breaking +changes are allowed in 0.x minors, so pinning an exact version is the right +move for production themes until 1.0. diff --git a/bin/pusha.js b/bin/pusha.js index 6baa12b..a576c95 100755 --- a/bin/pusha.js +++ b/bin/pusha.js @@ -1199,6 +1199,188 @@ function detectLiquidPersistentState(file, themePath) { return findings; } +// ─── Bucket R: island candidates ──────────────────────────────────────────── +// Stale-prone regions INSIDE the swap container. Bucket L is the same question +// asked about the persistent shell; R is the in-container answer, and the two +// do not overlap — L findings freeze because the shell is never swapped, R +// findings go stale because the page HTML came out of the prefetch cache. +// +// The remedy is opt-in markup, never inference. Pusha does not and must not +// guess which regions are stale-prone: revalidating a section costs a request +// and re-running its JS, so a wrong guess is a tax on every cached navigation. +// This bucket produces CANDIDATES for a human or agent to accept or decline. +// +// Ranks: `mark` — clear candidate, add the attributes. `ask` — plausible but +// the call depends on the merchant (a price that never changes is not stale). +// `ok` — already marked and correctly wired. +const R_PATTERNS = [ + // R-A price — the canonical island. Prefetch TTLs are minutes; a price + // change inside that window is served stale from cache. + { sub: 'A', rank: 'mark', re: /\|\s*money(_with_currency|_without_currency|_without_trailing_zeros)?\b/, what: 'money filter' }, + { sub: 'A', rank: 'mark', re: /\b(compare_at_price|price_varies|price_min|price_max)\b/, what: 'price comparison/range' }, + + // R-B availability — the one that costs a sale. A sold-out variant served + // from cache lets a buyer add to cart and fail at checkout. + { sub: 'B', rank: 'mark', re: /\b(available|sold_out)\b/, what: 'availability' }, + { sub: 'B', rank: 'mark', re: /\binventory_(quantity|policy|management)\b/, what: 'inventory.*' }, + + // R-C variant state — selected variant drives price and availability, so a + // stale variant object is both of the above at once. + { sub: 'C', rank: 'mark', re: /\bselected_or_first_available_variant\b/, what: 'selected_or_first_available_variant' }, + { sub: 'C', rank: 'ask', re: /\bcurrent_variant\b/, what: 'current_variant' }, + + // R-D cart-derived inside the container. Already covered by cart:mutated + // invalidating the cache, so this is a verify rather than a fix — listed + // because a theme that dispatches no cart:mutated has neither mechanism. + { sub: 'D', rank: 'ok', re: /\bcart\.(item_count|total_price|items_subtotal_price)\b/, what: 'cart.* in container' }, + + // R-E render-time values. `now` is the clearest case in the language: the + // value is wrong the moment it is cached. + { sub: 'E', rank: 'mark', re: /['"]now['"]\s*\|\s*date\b/, what: "'now' | date" }, +]; + +// A marker that cannot work. Each of these renders without error and silently +// never revalidates, which is the failure mode this bucket exists to prevent — +// islands shipped for a whole release with none of these checks and nobody +// noticed, because nothing throws. +function checkIslandMarkers(file, themePath, loc) { + const rel = relative(themePath, file); + const text = stripLiquidComments(readFileText(file)); + const lines = text.split('\n'); + const out = []; + for (let i = 0; i < lines.length; i++) { + const ln = lines[i]; + if (!/\bdata-island\b/.test(ln)) continue; + const line = i + 1; + const match = ln.trim().slice(0, 160); + + // The attributes may be spread over several lines of one tag, so look at a + // small window rather than the single line. + const windowText = lines.slice(Math.max(0, i - 6), i + 7).join('\n'); + const hasSectionId = /\bdata-section-id\s*=/.test(windowText); + + if (!hasSectionId) { + out.push({ file: rel, line, match, sub: 'marker', rank: 'gap', location: loc, + what: 'data-island with no data-section-id — findIslands matches [data-island][data-section-id], so this is inert' }); + continue; + } + if (loc === 'shell') { + out.push({ file: rel, line, match, sub: 'marker', rank: 'gap', location: loc, + what: 'data-island in the persistent shell — islands are collected from the swapped container only, so this never revalidates' }); + continue; + } + // `{% render %}` does not inherit the caller's scope. A snippet writing + // `{{ section.id }}` without being passed `section` renders an EMPTY id, + // and an empty data-section-id fails the attribute selector silently. + if (loc === 'include' && /\bdata-section-id\s*=\s*["']\{\{-?\s*section\.id/.test(windowText)) { + // Verify before reporting. The snippet is only broken if a caller fails + // to pass `section:` — a correctly-wired island would otherwise be + // reported as dead, which is the same error as suppressing on shape, + // inverted. + const snippetName = rel.replace(/^snippets\//, '').replace(/\.liquid$/, ''); + const bad = renderSitesMissingSection(themePath, snippetName); + if (bad.length) { + out.push({ file: rel, line, match, sub: 'marker', rank: 'gap', location: loc, + what: `snippet writes {{ section.id }} and {% render %} does not inherit scope — these render sites pass no \`section:\`, so the id renders empty there: ${bad.join(', ')}` }); + continue; + } + } + out.push({ file: rel, line, match, sub: 'marker', rank: 'ok', location: loc, + what: 'island marker present with a section id' }); + } + return out; +} + +function detectIslandCandidates(themePath, shellRelSet) { + const findings = []; + const dirs = ['sections', 'blocks', 'snippets', 'templates']; + for (const dir of dirs) { + const abs = join(themePath, dir); + if (!existsSync(abs)) continue; + for (const file of walkFiles(abs, ['.liquid'])) { + const rel = relative(themePath, file); + const loc = locationClass(rel, shellRelSet); + + // Marker conformance runs everywhere, including the shell — a marker in + // the shell is precisely one of the things worth reporting. + findings.push(...checkIslandMarkers(file, themePath, loc)); + + // Candidates are an in-container question only. Shell staleness is + // bucket L and has different remedies. + if (loc === 'shell') continue; + + const text = stripLiquidComments(readFileText(file)); + if (/\bdata-island\b/.test(text)) continue; // already marked — no candidate noise + const lines = text.split('\n'); + const seen = new Set(); + for (let i = 0; i < lines.length; i++) { + const ln = lines[i]; + for (const pat of R_PATTERNS) { + if (!pat.re.test(ln)) continue; + // One finding per file+sub. A product template mentions `money` + // fifteen times and they are all the same decision. + const key = `${rel}:${pat.sub}`; + if (seen.has(key)) continue; + seen.add(key); + findings.push({ + file: rel, + line: i + 1, + match: ln.trim().slice(0, 160), + sub: pat.sub, + rank: pat.rank, + location: loc, + what: pat.what, + sectionIdInScope: sectionIdResolvable(text, loc), + }); + } + } + } + } + return findings; +} + +// Can this file write a usable `data-section-id`? Sections and theme blocks get +// `section` from the renderer. Snippets do not — verified against a Horizon +// port, where 13 of 93 blocks read `section.id` happily while snippets have to +// be handed it at the render site. +function sectionIdResolvable(text, loc) { + if (loc === 'section' || loc === 'block') return true; + if (loc === 'include') return /\bsection\.id\b/.test(text) ? 'passed-in-required' : false; + return false; +} + +// Which render sites of this snippet fail to pass `section:`? Verified rather +// than assumed: a snippet reading `{{ section.id }}` is only broken if the +// render tag does not hand it `section`, and flagging every such snippet on +// shape alone reports a working island as broken. Returns the offending sites, +// so an empty array means every caller is correct. +// One static pattern for every render tag, comparing the captured name rather +// than building a regex per snippet. Two reasons: a snippet name goes straight +// into the pattern otherwise, which is both an injection shape and simply wrong +// for any name carrying a regex metacharacter; and the per-snippet version +// re-walked the whole theme once per marker. +const RENDER_TAG_WITH_ARGS = /\{%-?\s*(?:render|include)\s+['"]([\w.-]+)['"]([^%]*)-?%\}/g; + +function renderSitesMissingSection(themePath, snippetName) { + const missing = []; + for (const dir of ['sections', 'blocks', 'snippets', 'layout', 'templates']) { + const abs = join(themePath, dir); + if (!existsSync(abs)) continue; + for (const file of walkFiles(abs, ['.liquid'])) { + const text = stripLiquidComments(readFileText(file)); + RENDER_TAG_WITH_ARGS.lastIndex = 0; + let m; + while ((m = RENDER_TAG_WITH_ARGS.exec(text)) !== null) { + if (m[1] !== snippetName) continue; + if (/\bsection\s*:/.test(m[2] ?? '')) continue; + const line = text.slice(0, m.index).split('\n').length; + missing.push(`${relative(themePath, file)}:${line}`); + } + } + } + return missing; +} + // ─── Bucket M: persistent-shell stateful UI ───────────────────────────────── // Modals, drawers, overlays, and dropdowns that live OUTSIDE #MainContent and // were authored assuming a full reload would dismiss them. PJAX swaps don't @@ -1333,7 +1515,7 @@ function auditTheme(themePath, { useWhitelists = true } = {}) { if (!existsSync(dirs[name])) console.error(`! ${name}/ not found at ${dirs[name]} — skipping`); } - const findings = { A: [], B: [], C: [], D: [], E: [], F: [], G: [], H: [], J: [], K: [], L: [], M: [], P: [], X: [], unknown: [] }; + const findings = { A: [], B: [], C: [], D: [], E: [], F: [], G: [], H: [], J: [], K: [], L: [], M: [], P: [], R: [], X: [], unknown: [] }; // Each entry: { bucket, file, line?, match?, reason? } — the would-have-been // finding plus why it was suppressed. `files` keyed by Pusha-self filenames; // `G` keyed by the file-level Pusha.on* trigger; `H` keyed by line patterns. @@ -1536,6 +1718,12 @@ function auditTheme(themePath, { useWhitelists = true } = {}) { // location class so the report prescribes a paradigm-correct fix instead of a // blanket "wrap in sectionInits". const shellRelSet = new Set(shellFiles.map((f) => relative(themePath, f))); + + // R — island candidates. Runs after the shell is resolved because the whole + // question is "is this inside the swap container", and that is what the shell + // resolution answers. + findings.R = detectIslandCandidates(themePath, shellRelSet); + for (const f of findings.E) { let loc = locationClass(f.file, shellRelSet); if (loc === 'shell' && inlineScriptIsConfigOnly(join(themePath, f.file), f.line)) loc = 'head-config'; @@ -1638,6 +1826,11 @@ function auditTheme(themePath, { useWhitelists = true } = {}) { M_body_classes: findings.M.filter((x) => x.kind === 'body-class').length, P: findings.P.length, P_gaps: findings.P.filter((x) => x.dangling === 'undeclared').length, + R: findings.R.length, + R_mark: findings.R.filter((x) => x.rank === 'mark').length, + R_ask: findings.R.filter((x) => x.rank === 'ask').length, + R_gaps: findings.R.filter((x) => x.rank === 'gap').length, + R_marked: findings.R.filter((x) => x.sub === 'marker' && x.rank === 'ok').length, J: findings.J.length, J_gaps: findings.J.filter((x) => x.rank === 'gap').length, J_warns: findings.J.filter((x) => x.rank === 'warn').length, @@ -2130,6 +2323,7 @@ const BUCKET_RULES = { L: 'Per-request Liquid in the layout shell (layout/theme.liquid, section groups, transitively-rendered snippets) freezes on first load. Sub-letters mirror the request-scoped taxonomy: A=URL/template, B=customer, C=cart, D=locale, E=per-page object, F=personalization, G=time, H=app-injected. Rank: auto=URL-derivable in JS, ask=user decides (full-reload boundary or section refetch), ok=already handled by Pusha or theme convention.', M: 'Persistent-shell stateful UI — modals/drawers/overlays that lived outside #MainContent and were authored assuming a full reload would dismiss them. Three remediation options: (1) add `data-pusha-close-on-nav` to the root (Pusha strips `[open]` / sets `aria-expanded="false"` / removes body classes listed in `data-pusha-body-class-on-open`); (2) implement a `closeOnNav()` method on the custom element; (3) call `Pusha.onBeforeLeave(() => this.close())` manually. Cart drawers and persistent widgets simply omit the marker — opt-in is the safe default.', J: 'Analytics surface. NOTE: Pusha reaches the pixel sandbox only through PREFIXED CUSTOM events — the customEvents bridge publishes pusha:page_viewed plus prefixed copies of the page-type payloads, and those are delivered to custom pixels and app pixels. What is fenced is publishing under STANDARD names, which the storefront API rejects, so a standard page_viewed never arrives on a swap. Delivery is not consumption: a third-party app pixel subscribed to the standard vocabulary has no mapping for a prefixed name, so reviving it still needs a companion custom pixel that forwards the event (docs/analytics-companion-pixel.md, README "Analytics & tracking"). This bucket checks the theme-serialized