From 3da6d5aa31e4a464eb0bc027ec0e1624fa84d4fd Mon Sep 17 00:00:00 2001 From: Devin Michael Date: Thu, 3 Sep 2026 11:07:35 +0700 Subject: [PATCH 1/4] Render {% pixels %} in the base layout so app event trackers load The platform injects every app's storefront event tracker (GA4, GTM, Klaviyo, Taboola) as hidden iframes through the builtin {% pixels %} tag. Intro Bootstrap reaches it indirectly through {% core_js %}; Spark replaced core_js with spark-platform.js and never re-added pixels, so no tracker iframe rendered, window.customerEventManager never existed, and no app received any storefront event on Spark-based stores. Add a `pixels` block before the theme script stack so the tag's fetch hook is installed before spark-cart and friends make requests. Document the block in the load-order contract, CLAUDE.md, and the changelog. Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 1 + CLAUDE.md | 2 +- docs/performance-load-order.md | 6 ++++-- layouts/base.html | 5 +++++ 4 files changed, 11 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8112f19..3a0fcf6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ Spark follows human-readable release notes rather than a package-manager version ## Unreleased +- `layouts/base.html` now renders the platform `{% pixels %}` tag in a new `pixels` block, immediately before the theme script stack. The platform injects every app's storefront event tracker (Google Analytics 4, Google Tag Manager, Klaviyo, Taboola) as hidden iframes through that tag, and Intro Bootstrap reaches it indirectly through `{% core_js %}`. Spark replaced `core_js` with `spark-platform.js` and never re-added `pixels`, so no tracker iframe was rendered, `window.customerEventManager` never existed, and no app received `page_viewed`, `product_viewed`, `product_added_to_cart`, or `checkout_*` events on any Spark store. The tag is a platform builtin, needs no `{% load %}`, does not depend on jQuery, and respects the platform's `disable_pixel` guard. - `scripts/sass-compat.py --check` now rejects function names the platform's Sass pass claims as its own colour built-ins, under two rules that differ in what the author should do. `sass-builtin-as-css-filter` covers `invert()`, `saturate()`, `grayscale()`, and `opacity()`: these are real CSS filter functions, so a valid `filter: brightness(0) invert(1)` fails the upload with "Could not compile CSS. Please check Scss Syntax." while `make css-check` passes locally; use a filter function Sass does not claim, or assign through a custom property. `sass-colour-function-in-css` covers `lighten()`, `darken()`, `complement()`, and `desaturate()`, which are not CSS functions at all and mean Sass source reached the generated output; emit the computed colour instead. Custom-property declarations are exempt from both, because Sass leaves a custom property's value alone; that is why Tailwind's own `--tw-grayscale: grayscale(100%)` compiles and stays accepted. - Rebuilt `assets/main.css`. Tailwind scans the repo's own docs and scripts for class candidates, so naming the banned functions in this change's prose generates an unused `.invert` utility. The committed CSS has to match a fresh build for the `css-drift` gate, so the rebuild ships with the change. diff --git a/CLAUDE.md b/CLAUDE.md index 593104d..1481e52 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -22,7 +22,7 @@ Spark is a modern starter theme for Next Commerce storefronts. Tailwind CSS + va - **Performance load order:** `layouts/base.html` separates metadata, LCP preloads, CSS, rare priority JS, body content, ordered theme JS, footer app hooks, and tracking. See `docs/performance-load-order.md` before adding head scripts or preloads. - **Templates:** Django Template Language (DTL) - **Icons:** SVG partials in `partials/icons/` -- **jQuery:** REMOVED. Zero jQuery, zero Bootstrap. `spark-platform.js` replaces `{% core_js %}` with vanilla JS. +- **jQuery:** REMOVED. Zero jQuery, zero Bootstrap. `spark-platform.js` replaces `{% core_js %}` with vanilla JS. The one piece of `core_js` that Spark still needs is the platform's `{% pixels %}` tag (app event-tracker iframes + `customerEventManager`); `layouts/base.html` renders it in its own `pixels` block before the theme script stack. Do not remove it. - **Design System:** See [DESIGN.md](DESIGN.md) for all visual decisions. ## CSS Pipeline diff --git a/docs/performance-load-order.md b/docs/performance-load-order.md index cfc622c..cadbe27 100644 --- a/docs/performance-load-order.md +++ b/docs/performance-load-order.md @@ -15,8 +15,9 @@ The goal is simple: the browser should discover metadata, LCP assets, and CSS be 5. `extrahead` and `head_app_hooks` for platform-critical head snippets. 6. Body content through `content_wrapper`. 7. Non-critical UI shells such as the side cart. -8. Minimal global JavaScript: `SparkEvents`, `SparkCartLoader`, `theme.js`, and platform compatibility. -9. Page `component_scripts`, page `extrascripts`, inline Spark enhancements, footer app hooks, and `tracking`. +8. `pixels`: the platform's `{% pixels %}` tag, which renders one hidden iframe per installed app event tracker (GA4, GTM, Klaviyo, Taboola) and `customerEventManager`. It runs before the theme script stack so its `fetch` hook is in place before `spark-cart` and friends make requests. Intro Bootstrap gets this through `{% core_js %}`; Spark dropped `core_js` and renders the tag itself. +9. Minimal global JavaScript: `SparkEvents`, `SparkCartLoader`, `theme.js`, and platform compatibility. +10. Page `component_scripts`, page `extrascripts`, inline Spark enhancements, footer app hooks, and `tracking`. ## Theme-Side Rules @@ -45,6 +46,7 @@ Other templates should add a `critical_preloads` override only when they have a Spark's global layout loads only: +- the platform `{% pixels %}` output (`pixels.min.js` plus the app tracker iframes; not a theme asset) - `spark-events.js` - `spark-cart-loader.js` - `theme.js` diff --git a/layouts/base.html b/layouts/base.html index 715fe5f..604bdf1 100644 --- a/layouts/base.html +++ b/layouts/base.html @@ -169,6 +169,11 @@ {% include 'partials/side_cart.html' %} {% endblock side_cart %} + {# Platform event trackers (GA4, GTM, Klaviyo, Taboola apps subscribe through these frames). Rendered by core_js in Intro Bootstrap; Spark dropped core_js and must render it itself. #} + {% block pixels %} + {% pixels %} + {% endblock pixels %} + {# Theme JS - dependency ordered, zero jQuery, zero Bootstrap. #} {% block scripts %} From 905e3767bb3e052270b2814a36af3a41a5a9a8f4 Mon Sep 17 00:00:00 2001 From: Devin Michael Date: Thu, 3 Sep 2026 11:48:43 +0700 Subject: [PATCH 2/4] Release 1.3.0 Bump the version markers in manifest.json, README.md, and CLAUDE.md, and promote the Unreleased changelog section to a dated 1.3.0 heading. Adds entries for the merged-but-unlogged changes since 1.2.0: the filter-argument escape check (#52), the scoped Tailwind content scan (#53), and the __pycache__ ignore (#54). Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 5 ++++- CLAUDE.md | 2 +- README.md | 2 +- manifest.json | 2 +- 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a0fcf6..43bc3ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,11 +4,14 @@ All notable public Spark changes should be recorded here. Spark follows human-readable release notes rather than a package-manager version contract. The release version is stored in `manifest.json` and mirrored in [README.md](README.md) and `CLAUDE.md`. When releasing, update all three version markers, add a dated changelog section, and publish a Git tag or GitHub release when the repo is ready for external consumers to pin versions. -## Unreleased +## 1.3.0 - 2026-09-03 - `layouts/base.html` now renders the platform `{% pixels %}` tag in a new `pixels` block, immediately before the theme script stack. The platform injects every app's storefront event tracker (Google Analytics 4, Google Tag Manager, Klaviyo, Taboola) as hidden iframes through that tag, and Intro Bootstrap reaches it indirectly through `{% core_js %}`. Spark replaced `core_js` with `spark-platform.js` and never re-added `pixels`, so no tracker iframe was rendered, `window.customerEventManager` never existed, and no app received `page_viewed`, `product_viewed`, `product_added_to_cart`, or `checkout_*` events on any Spark store. The tag is a platform builtin, needs no `{% load %}`, does not depend on jQuery, and respects the platform's `disable_pixel` guard. - `scripts/sass-compat.py --check` now rejects function names the platform's Sass pass claims as its own colour built-ins, under two rules that differ in what the author should do. `sass-builtin-as-css-filter` covers `invert()`, `saturate()`, `grayscale()`, and `opacity()`: these are real CSS filter functions, so a valid `filter: brightness(0) invert(1)` fails the upload with "Could not compile CSS. Please check Scss Syntax." while `make css-check` passes locally; use a filter function Sass does not claim, or assign through a custom property. `sass-colour-function-in-css` covers `lighten()`, `darken()`, `complement()`, and `desaturate()`, which are not CSS functions at all and mean Sass source reached the generated output; emit the computed colour instead. Custom-property declarations are exempt from both, because Sass leaves a custom property's value alone; that is why Tailwind's own `--tw-grayscale: grayscale(100%)` compiles and stays accepted. - Rebuilt `assets/main.css`. Tailwind scans the repo's own docs and scripts for class candidates, so naming the banned functions in this change's prose generates an unused `.invert` utility. The committed CSS has to match a fresh build for the `css-drift` gate, so the rebuild ships with the change. +- `scripts/check-templates.py` now rejects unsupported backslash escapes inside quoted template filter arguments. Django only understands `\"` and `\\` there, so `split:"\n"` silently split on the two literal characters instead of a newline; the platform renders it without error and the defect only shows once real settings data flows through. Use `|linebreaksbr|split:"
"` for the newline case. Comments and verbatim blocks stay exempt (#52). +- Tailwind's content scan is now limited to the directories that carry class names (`layouts`, `templates`, `partials`, `assets/js`) via `source(none)` in `css/input.css`. Prose in docs, scripts, and tests no longer compiles stray utilities into `assets/main.css`; twelve unused ones were removed from the shipped CSS. A new directory containing class attributes generates nothing until it is added to the list, and `tests/test_tailwind_source_guard.py` pins that list so the omission fails a test instead of silently dropping storefront styles (#53). +- Python bytecode from the tooling tests is now gitignored, so `make test` and `make verify-theme` no longer leave untracked `__pycache__` directories behind (#54). ## 1.2.0 - 2026-09-01 diff --git a/CLAUDE.md b/CLAUDE.md index 1481e52..b34a2f2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -3,7 +3,7 @@ ## Overview Spark is a modern starter theme for Next Commerce storefronts. Tailwind CSS + vanilla JS. Clean, minimal commerce aesthetic. Intended to replace Intro Bootstrap as the default starter theme and become a product in its own right. -**Current version:** 1.2.0 +**Current version:** 1.3.0 **Repo:** `NextCommerceCo/spark` (public starter theme) **ntk config:** `config.yml` is gitignored and store-specific. Create it with `ntk init`; never commit store credentials. **Companion skill:** Use the [next-theme-dev skill](https://github.com/NextCommerceCo/skills/tree/main/next-theme-dev) for AI-assisted Spark, Intro Bootstrap, and custom Next Commerce theme work. diff --git a/README.md b/README.md index 33b7598..79958d6 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ A modern starter theme for Next Commerce. Tailwind CSS v4, vanilla JS + Web Components, zero jQuery, zero Bootstrap, no bundler required. -**Current version:** 1.2.0 +**Current version:** 1.3.0 **Release status:** Public starter theme. Spark is installable on NEXT stores via `ntk` and is used for public-facing storefront builds. The theme is ready for theme developers to clone, inspect, adapt, and push to stores they control. diff --git a/manifest.json b/manifest.json index c1ccc9d..6b1328e 100644 --- a/manifest.json +++ b/manifest.json @@ -1,4 +1,4 @@ { "name": "Spark", - "version": "1.2.0" + "version": "1.3.0" } From 1df91213f27b2d3efd795c173705815e7ee6ee5f Mon Sep 17 00:00:00 2001 From: Devin Michael Date: Thu, 3 Sep 2026 11:52:16 +0700 Subject: [PATCH 3/4] spark-cart: recover when the stored cart was consumed by checkout Reproduced on aptest.29next.store on 2026-09-03 right after an order completed: every addCartLines afterwards returned HTTP 200 with {"addCartLines":{"success":false,"errors":{"nonFieldErrors": [[{"message":"Cart not found.","code":"cart_not_found"}]]},"cart":null}} and the shopper could not add anything until browser storage was cleared. Two things combined. The consumed cart id stays in sessionStorage and the storefront_cart_id cookie because the order-confirmation page is rendered by the platform, not the theme, so nothing on the theme side ever ran to forget it. And addToCart only recovered when the request *rejected* and the message matched isCartExpiredError; this payload resolves, so the .then branch handed back success:false and the create-and-retry path never ran. - isCartNotFoundResult() recognises the resolved shape (code cart_not_found, or a message matching the existing expired-cart heuristic, at any depth of the errors object). - addToCart settles the request into an outcome first, then recovers exactly once for either a rejection or a resolved cart_not_found: clear the stored id, createCart, retry with recover=false. A failure inside the retry cannot re-enter the recovery branch. - getCart clears the stored id when the platform returns cart:null or a not-found error. Badge hydration calls this on the first storefront page after confirmation, so the stale id is gone before the shopper's next add. - updateCartLines, removeCartLines, addVoucher, removeVoucher clear the id on cart_not_found without recreating (the caller is editing a cart it believes exists), so a stale id cannot leak into the next add. - clearCartId() removes the sessionStorage key and expires the cookie. Tests: the exact aptest payload on the first add asserts createCart and a second addCartLines follow with the new id stored; a second miss returns the platform answer without looping; getCart null/reject and each explicit-id mutation clear the stored id and dispatch nothing. Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 1 + assets/js/spark-cart.js | 131 ++++++++++++++++++++++------- tests/js/spark-cart-client.test.js | 100 ++++++++++++++++++++++ 3 files changed, 203 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 43bc3ec..020f692 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ Spark follows human-readable release notes rather than a package-manager version - `layouts/base.html` now renders the platform `{% pixels %}` tag in a new `pixels` block, immediately before the theme script stack. The platform injects every app's storefront event tracker (Google Analytics 4, Google Tag Manager, Klaviyo, Taboola) as hidden iframes through that tag, and Intro Bootstrap reaches it indirectly through `{% core_js %}`. Spark replaced `core_js` with `spark-platform.js` and never re-added `pixels`, so no tracker iframe was rendered, `window.customerEventManager` never existed, and no app received `page_viewed`, `product_viewed`, `product_added_to_cart`, or `checkout_*` events on any Spark store. The tag is a platform builtin, needs no `{% load %}`, does not depend on jQuery, and respects the platform's `disable_pixel` guard. - `scripts/sass-compat.py --check` now rejects function names the platform's Sass pass claims as its own colour built-ins, under two rules that differ in what the author should do. `sass-builtin-as-css-filter` covers `invert()`, `saturate()`, `grayscale()`, and `opacity()`: these are real CSS filter functions, so a valid `filter: brightness(0) invert(1)` fails the upload with "Could not compile CSS. Please check Scss Syntax." while `make css-check` passes locally; use a filter function Sass does not claim, or assign through a custom property. `sass-colour-function-in-css` covers `lighten()`, `darken()`, `complement()`, and `desaturate()`, which are not CSS functions at all and mean Sass source reached the generated output; emit the computed colour instead. Custom-property declarations are exempt from both, because Sass leaves a custom property's value alone; that is why Tailwind's own `--tw-grayscale: grayscale(100%)` compiles and stays accepted. - Rebuilt `assets/main.css`. Tailwind scans the repo's own docs and scripts for class candidates, so naming the banned functions in this change's prose generates an unused `.invert` utility. The committed CSS has to match a fresh build for the `css-drift` gate, so the rebuild ships with the change. +- Add-to-cart no longer stays dead after a purchase. Checkout consumes the cart but the theme kept its id in `sessionStorage` and the `storefront_cart_id` cookie, and the platform answers a consumed id with a resolved `success: false` / `cart_not_found` payload rather than a rejected request, so `SparkCartClient.addToCart` never took its create-and-retry path: every add failed until the shopper cleared browser storage. `addToCart` now treats that payload like an expired cart (forget the id, `createCart`, retry once). `getCart` forgets the id when the platform no longer knows the cart, which is what the cart badge asks on the first storefront page after the platform's order-confirmation page, and the line, quantity, and voucher mutations forget it too so a stale id cannot leak into the next add. Reproduced on aptest.29next.store on 2026-09-03. - `scripts/check-templates.py` now rejects unsupported backslash escapes inside quoted template filter arguments. Django only understands `\"` and `\\` there, so `split:"\n"` silently split on the two literal characters instead of a newline; the platform renders it without error and the defect only shows once real settings data flows through. Use `|linebreaksbr|split:"
"` for the newline case. Comments and verbatim blocks stay exempt (#52). - Tailwind's content scan is now limited to the directories that carry class names (`layouts`, `templates`, `partials`, `assets/js`) via `source(none)` in `css/input.css`. Prose in docs, scripts, and tests no longer compiles stray utilities into `assets/main.css`; twelve unused ones were removed from the shipped CSS. A new directory containing class attributes generates nothing until it is added to the list, and `tests/test_tailwind_source_guard.py` pins that list so the omission fails a test instead of silently dropping storefront styles (#53). - Python bytecode from the tooling tests is now gitignored, so `make test` and `make verify-theme` no longer leave untracked `__pycache__` directories behind (#54). diff --git a/assets/js/spark-cart.js b/assets/js/spark-cart.js index 22fb6b6..5b2ce13 100644 --- a/assets/js/spark-cart.js +++ b/assets/js/spark-cart.js @@ -58,6 +58,19 @@ setCookie(CART_ID_KEY, id, 30); } + /** + * Forget the stored cart id. Called once the platform reports the cart is + * gone, so the next addToCart creates a fresh cart instead of retrying a + * consumed one. Checkout completion does not clear this on its own: the + * order-confirmation page is rendered by the platform, not the theme. + */ + function clearCartId() { + try { + sessionStorage.removeItem(CART_ID_KEY); + } catch(e) {} + setCookie(CART_ID_KEY, '', -1); + } + /* --- Currency formatting --- */ function formatMoney(amount, currency) { @@ -221,6 +234,49 @@ msg.indexOf('invalid') !== -1; } + /** + * Helper: check if a *resolved* mutation result reports a missing cart. + * The platform answers a consumed cart id with HTTP 200 and + * `{ success: false, errors: { nonFieldErrors: [[{ code: 'cart_not_found' }]] }, cart: null }`, + * which never reaches the rejection path that isCartExpiredError guards. + * Walks whatever shape `errors` takes (object, nested arrays, strings). + */ + function isCartNotFoundResult(result) { + if (!result || result.success !== false || !result.errors) return false; + var found = false; + (function walk(node, depth) { + if (found || node == null || depth > 6) return; + if (typeof node === 'string') { + found = isCartExpiredError({ message: node }); + return; + } + if (typeof node !== 'object') return; + if (node.code === 'cart_not_found' || (node.message && isCartExpiredError(node))) { + found = true; + return; + } + var keys = Object.keys(node); + for (var i = 0; i < keys.length && !found; i++) walk(node[keys[i]], depth + 1); + })(result.errors, 0); + return found; + } + + /** + * Helper: shared post-mutation handling. Clears the stored id when the + * platform says the cart is gone; otherwise persists the id and notifies. + */ + function noteMutationResult(client, result, action) { + if (isCartNotFoundResult(result)) { + clearCartId(); + return result; + } + if (result && result.cart) { + setCartId(result.cart.id); + client._dispatchCartUpdated(result.cart, action); + } + return result; + } + /** * Create a new empty cart. Stores the cart ID for future requests. * @returns {Promise} Full cart object @@ -244,9 +300,18 @@ var id = cartId || getCartId(); if (!id) return Promise.resolve(null); return this._request(GET_CART, { id: id }).then(function(data) { - return data.cart || null; + if (!data.cart) { + // The platform no longer knows this cart (typically consumed by + // checkout). Forget it so the next add starts a fresh cart. + clearCartId(); + return null; + } + return data.cart; }).catch(function(err) { - if (isCartExpiredError(err)) return null; + if (isCartExpiredError(err)) { + clearCartId(); + return null; + } throw err; }); }; @@ -263,7 +328,14 @@ var self = this; quantity = quantity || 1; - function doAdd(cartId) { + function recreateAndRetry() { + clearCartId(); + return self.createCart().then(function(cart) { + return doAdd(cart.id, false); + }); + } + + function doAdd(cartId, recover) { var lineInput = { productPk: productPk, quantity: quantity }; if (isUpsell) lineInput.isUpsell = true; @@ -289,30 +361,33 @@ cartId: cartId, lines: [lineInput] }; + // Settle the request into an outcome first so the recovery below + // runs exactly once: a failure inside recreateAndRetry must not + // re-enter this branch. return self._request(ADD_CART_LINES, { input: input }).then(function(data) { - var result = data.addCartLines; - if (result && result.cart) { - setCartId(result.cart.id); - self._dispatchCartUpdated(result.cart, 'add'); + return { data: data }; + }, function(err) { + return { err: err }; + }).then(function(outcome) { + if (outcome.err) { + if (recover && isCartExpiredError(outcome.err)) return recreateAndRetry(); + throw outcome.err; } - return result; + var result = outcome.data.addCartLines; + // The platform answers a consumed cart id with a resolved + // `success: false` / `cart_not_found` payload, not a rejection. + if (recover && isCartNotFoundResult(result)) return recreateAndRetry(); + return noteMutationResult(self, result, 'add'); }); } var cartId = getCartId(); if (cartId) { - return doAdd(cartId).catch(function(err) { - if (isCartExpiredError(err)) { - return self.createCart().then(function(cart) { - return doAdd(cart.id); - }); - } - throw err; - }); + return doAdd(cartId, true); } return this.createCart().then(function(cart) { - return doAdd(cart.id); + return doAdd(cart.id, false); }); }; @@ -326,12 +401,7 @@ var self = this; var input = { cartId: cartId, lines: lines }; return this._request(UPDATE_CART_LINES, { input: input }).then(function(data) { - var result = data.updateCartLines; - if (result && result.cart) { - setCartId(result.cart.id); - self._dispatchCartUpdated(result.cart, 'update'); - } - return result; + return noteMutationResult(self, data.updateCartLines, 'update'); }); }; @@ -345,12 +415,7 @@ var self = this; var input = { cartId: cartId, lineIds: lineIds }; return this._request(REMOVE_CART_LINES, { input: input }).then(function(data) { - var result = data.removeCartLines; - if (result && result.cart) { - setCartId(result.cart.id); - self._dispatchCartUpdated(result.cart, 'remove'); - } - return result; + return noteMutationResult(self, data.removeCartLines, 'remove'); }); }; @@ -365,6 +430,10 @@ var input = { cartId: cartId, vouchers: [code] }; return this._request(ADD_VOUCHER, { input: input }).then(function(data) { var result = data.addVoucher; + if (isCartNotFoundResult(result)) { + clearCartId(); + return result; + } if (result && result.cart) { self._dispatchCartUpdated(result.cart, 'voucher_add'); } @@ -383,6 +452,10 @@ var input = { cartId: cartId, vouchers: [voucherCode] }; return this._request(REMOVE_VOUCHER, { input: input }).then(function(data) { var result = data.removeVoucher; + if (isCartNotFoundResult(result)) { + clearCartId(); + return result; + } if (result && result.cart) { self._dispatchCartUpdated(result.cart, 'voucher_remove'); } diff --git a/tests/js/spark-cart-client.test.js b/tests/js/spark-cart-client.test.js index a8925ea..64a1b25 100644 --- a/tests/js/spark-cart-client.test.js +++ b/tests/js/spark-cart-client.test.js @@ -44,6 +44,10 @@ function createEnvironment(options = {}) { setItem: function(key, value) { if (options.storageThrows) throw new Error('storage unavailable'); storage[key] = String(value); + }, + removeItem: function(key) { + if (options.storageThrows) throw new Error('storage unavailable'); + delete storage[key]; } }; @@ -376,6 +380,99 @@ async function testStaticUtilities() { assert.equal(env.Client.MAX_QTY_PER_LINE, 15); } +// Exact payload the platform returned on aptest.29next.store on 2026-09-03 for +// every addCartLines after checkout consumed the stored cart: HTTP 200, no +// top-level GraphQL errors, so _request resolves instead of rejecting. +const CART_NOT_FOUND_RESULT = { + success: false, + errors: { nonFieldErrors: [[{ message: 'Cart not found.', code: 'cart_not_found' }]] }, + cart: null +}; + +async function testAddToCartRecoversFromResolvedCartNotFound() { + const env = createEnvironment({ cookies: { storefront_cart_id: 'consumed-cart' } }); + env.storage.storefront_cart_id = 'consumed-cart'; + const client = new env.Client(); + const calls = []; + client._request = function(query, variables) { + if (query.indexOf('CreateCart') !== -1) { + calls.push('createCart'); + return Promise.resolve({ createCart: { cart: { id: 'fresh-cart' } } }); + } + calls.push('addCartLines:' + variables.input.cartId); + if (variables.input.cartId === 'consumed-cart') { + return Promise.resolve({ addCartLines: CART_NOT_FOUND_RESULT }); + } + return Promise.resolve({ addCartLines: { success: true, cart: { id: 'fresh-cart', numItems: 1 } } }); + }; + + const result = await client.addToCart(7, 1); + assert.equal(result.success, true); + assert.deepEqual(calls, ['addCartLines:consumed-cart', 'createCart', 'addCartLines:fresh-cart']); + assert.equal(env.storage.storefront_cart_id, 'fresh-cart'); + assert.equal(env.cookieJar.storefront_cart_id, 'fresh-cart'); + assert.equal(env.events.length, 1); + assert.equal(env.events[0].detail.action, 'add'); +} + +async function testAddToCartRecoveryRunsOnlyOnce() { + const env = createEnvironment({ cookies: { storefront_cart_id: 'consumed-cart' } }); + const client = new env.Client(); + let adds = 0; + client._request = function(query) { + if (query.indexOf('CreateCart') !== -1) { + return Promise.resolve({ createCart: { cart: { id: 'fresh-cart' } } }); + } + adds += 1; + return Promise.resolve({ addCartLines: CART_NOT_FOUND_RESULT }); + }; + + const result = await client.addToCart(7, 1); + assert.equal(result.success, false); + assert.equal(adds, 2, 'one recovery attempt, then the platform answer is returned as-is'); + assert.equal(env.storage.storefront_cart_id, undefined, 'a second miss leaves no stale id behind'); +} + +async function testGoneCartClearsStoredId() { + // getCart: the first storefront request after the platform's own + // order-confirmation page is badge hydration, which must forget the id. + const nullEnv = createEnvironment({ cookies: { storefront_cart_id: 'consumed-cart' } }); + nullEnv.storage.storefront_cart_id = 'consumed-cart'; + const nullClient = new nullEnv.Client(); + nullClient._request = function() { return Promise.resolve({ cart: null }); }; + assert.equal(await nullClient.getCart(), null); + assert.equal(nullClient.getCartId(), null); + assert.equal(nullEnv.storage.storefront_cart_id, undefined); + + const rejectEnv = createEnvironment({ cookies: { storefront_cart_id: 'consumed-cart' } }); + const rejectClient = new rejectEnv.Client(); + rejectClient._request = function() { return Promise.reject(new Error('Cart not found')); }; + assert.equal(await rejectClient.getCart(), null); + assert.equal(rejectClient.getCartId(), null); + + // Explicit-id mutations do not recreate (the caller is editing a cart it + // believes exists) but must stop the stale id from leaking into the next add. + for (const [method, field, args] of [ + ['updateCartLines', 'updateCartLines', ['consumed-cart', [{ lineId: '1', quantity: 2 }]]], + ['removeCartLines', 'removeCartLines', ['consumed-cart', ['1']]], + ['addVoucher', 'addVoucher', ['consumed-cart', 'SAVE']], + ['removeVoucher', 'removeVoucher', ['consumed-cart', 'SAVE']] + ]) { + const env = createEnvironment({ cookies: { storefront_cart_id: 'consumed-cart' } }); + env.storage.storefront_cart_id = 'consumed-cart'; + const client = new env.Client(); + client._request = function() { + const data = {}; + data[field] = CART_NOT_FOUND_RESULT; + return Promise.resolve(data); + }; + const result = await client[method].apply(client, args); + assert.equal(result.success, false, method); + assert.equal(client.getCartId(), null, method + ' clears the stored id'); + assert.equal(env.events.length, 0, method + ' dispatches nothing for a gone cart'); + } +} + const tests = [ ['createCart persistence paths', testCreateCartPersistencePaths], ['getCart outcomes', testGetCartOutcomes], @@ -383,6 +480,9 @@ const tests = [ ['zero quantity is promoted', testZeroQuantityIsPromoted], ['unvalidated quantities are forwarded', testUnvalidatedQuantitiesAreForwarded], ['addToCart expired retry', testAddToCartExpiredRetry], + ['addToCart recovers from resolved cart_not_found', testAddToCartRecoversFromResolvedCartNotFound], + ['addToCart recovery runs only once', testAddToCartRecoveryRunsOnlyOnce], + ['gone cart clears stored id', testGoneCartClearsStoredId], ['subscription validation and input', testSubscriptionValidationAndInput], ['mutation variables and events', testMutationVariablesAndEvents], ['request errors and retries', testRequestErrorsAndRetries], From 3d52e181b29b76ad3c81c5d56696b53b5d4a4e72 Mon Sep 17 00:00:00 2001 From: Devin Michael Date: Thu, 3 Sep 2026 12:13:21 +0700 Subject: [PATCH 4/4] =?UTF-8?q?spark-cart:=20only=20a=20cart=5Fnot=5Ffound?= =?UTF-8?q?=20code=20or=20"cart=20=E2=80=A6=20not=20found"=20marks=20a=20r?= =?UTF-8?q?esolved=20result?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding on #55: isCartNotFoundResult fell back to isCartExpiredError, whose substring match on "invalid" would have flagged ordinary resolved validation errors ("Invalid quantity", "Invalid voucher code") as a missing cart and wiped a live cart id. Resolved results now count only the cart_not_found code or a message naming the cart as not found; the broad matcher stays on the rejection path where it started. Regression test covers an add, a voucher, and a line update that each fail validation with "invalid" in the message and assert no recreate and the id kept. Co-Authored-By: Claude Fable 5.1 --- assets/js/spark-cart.js | 16 ++++++++++++++-- tests/js/spark-cart-client.test.js | 30 ++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/assets/js/spark-cart.js b/assets/js/spark-cart.js index 5b2ce13..70ff74c 100644 --- a/assets/js/spark-cart.js +++ b/assets/js/spark-cart.js @@ -240,18 +240,30 @@ * `{ success: false, errors: { nonFieldErrors: [[{ code: 'cart_not_found' }]] }, cart: null }`, * which never reaches the rejection path that isCartExpiredError guards. * Walks whatever shape `errors` takes (object, nested arrays, strings). + * + * Deliberately stricter than isCartExpiredError: resolved results also + * carry ordinary validation errors ("Invalid quantity", "Invalid voucher + * code"), and the broad 'invalid' substring there would wipe a live cart. + * Only the cart_not_found code, or a message naming the cart as not found, + * counts. */ + function isCartNotFoundMessage(message) { + if (typeof message !== 'string') return false; + var msg = message.toLowerCase(); + return msg.indexOf('cart') !== -1 && msg.indexOf('not found') !== -1; + } + function isCartNotFoundResult(result) { if (!result || result.success !== false || !result.errors) return false; var found = false; (function walk(node, depth) { if (found || node == null || depth > 6) return; if (typeof node === 'string') { - found = isCartExpiredError({ message: node }); + found = isCartNotFoundMessage(node); return; } if (typeof node !== 'object') return; - if (node.code === 'cart_not_found' || (node.message && isCartExpiredError(node))) { + if (node.code === 'cart_not_found' || isCartNotFoundMessage(node.message)) { found = true; return; } diff --git a/tests/js/spark-cart-client.test.js b/tests/js/spark-cart-client.test.js index 64a1b25..8403bd1 100644 --- a/tests/js/spark-cart-client.test.js +++ b/tests/js/spark-cart-client.test.js @@ -473,6 +473,35 @@ async function testGoneCartClearsStoredId() { } } +async function testValidationErrorsKeepTheStoredId() { + // Ordinary resolved validation errors carry no cart_not_found code and + // often contain "invalid"; they must not be mistaken for a missing cart. + const cases = [ + ['addCartLines', 'addToCart', [7, 1], { lines: [[{ message: 'Invalid quantity', code: 'invalid' }]] }], + ['addVoucher', 'addVoucher', ['live-cart', 'NOPE'], { vouchers: [[{ message: 'Invalid voucher code' }]] }], + ['updateCartLines', 'updateCartLines', ['live-cart', [{ lineId: '1', quantity: 99 }]], { nonFieldErrors: ['Invalid quantity for line 1'] }] + ]; + for (const [field, method, args, errors] of cases) { + const env = createEnvironment({ cookies: { storefront_cart_id: 'live-cart' } }); + env.storage.storefront_cart_id = 'live-cart'; + const client = new env.Client(); + let creates = 0; + client._request = function(query) { + if (query.indexOf('CreateCart') !== -1) { + creates += 1; + return Promise.resolve({ createCart: { cart: { id: 'unwanted-cart' } } }); + } + const data = {}; + data[field] = { success: false, errors: errors, cart: null }; + return Promise.resolve(data); + }; + const result = await client[method].apply(client, args); + assert.equal(result.success, false, method); + assert.equal(creates, 0, method + ' must not recreate the cart'); + assert.equal(client.getCartId(), 'live-cart', method + ' keeps the stored id'); + } +} + const tests = [ ['createCart persistence paths', testCreateCartPersistencePaths], ['getCart outcomes', testGetCartOutcomes], @@ -483,6 +512,7 @@ const tests = [ ['addToCart recovers from resolved cart_not_found', testAddToCartRecoversFromResolvedCartNotFound], ['addToCart recovery runs only once', testAddToCartRecoveryRunsOnlyOnce], ['gone cart clears stored id', testGoneCartClearsStoredId], + ['validation errors keep the stored id', testValidationErrorsKeepTheStoredId], ['subscription validation and input', testSubscriptionValidationAndInput], ['mutation variables and events', testMutationVariablesAndEvents], ['request errors and retries', testRequestErrorsAndRetries],