fix: resolve slug kind before fetching product detail - #2
Open
kalwiggins wants to merge 1 commit into
Open
Conversation
The catch-all [slug] route asked the expensive question first: every category and brand page view fired a guaranteed-404 product lookup, twice per request (generateMetadata and the page). Cheap here, where the default repositories read bundled JSON -- but a real API call in any storefront generated from this template with a live backend wired in. Measured on RVGearPro, generated from this template and pointed at Foundry: in one hour of CloudFront access logs, 6,703 of 6,709 catalog requests were exactly this, each one a real database query at the origin. categoryRepository.getBySlug and brandRepository.getBySlug resolve against a cached list -- an in-memory find, no network. productRepository.getBySlug is the live call. So ask the free questions first and only pay for the product lookup when the slug is neither, where a miss is a genuine 404. Precedence change: a slug matching BOTH a category and a product now resolves to the category. Preserving the old precedence would mean always paying for the product lookup, which is the cost being removed. Existing storefronts do not inherit this -- they were generated, not linked -- and are being fixed separately. This stops future ones being born with it.
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 free
to 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.
Fixes the source of a defect that has now propagated into every storefront generated from this template.
The problem
In the catch-all
[slug]route, every category and brand page view fires a guaranteed-404 product lookup first — twice per request, once ingenerateMetadataand once in the page. In this template that costs little, because the default repositories read bundled JSON. In a downstream storefront with a live backend wired in, each one becomes a real API call.Measured on RVGearPro, generated from this template and pointed at Foundry — one hour of CloudFront access logs:
~99.8% of its catalog API traffic achieved nothing, and each 404 was a real database query at the origin.
Root cause
The resolution order is backwards relative to cost:
categoryRepository.getBySlugfind, no networkbrandRepository.getBySlugfind, no networkproductRepository.getBySlugThe expensive question was asked first, unconditionally.
The fix
resolveSlugKind()asks the two free questions first and falls through to"product"only when the slug is neither — where a miss is a genuine 404. Both entry points share it, so the work happens once per request instead of twice.The diff is deliberately small: one line per branch, all rendering and SEO bodies untouched.
Precedence change
A slug matching both a category and a product now resolves to the category, where it previously resolved to the product. Preserving the old precedence would require always paying for the product lookup, which is the cost being removed. Flagging it because it is a behaviour change, not a pure optimization.
Downstream
Existing storefronts do not inherit this automatically — each was generated, not linked. The same fix has been raised separately against the ones with a live backend (Foundry, MiniPIM or BigCommerce). Storefronts wired only to static JSON were deliberately skipped: there is nothing to save there.
This PR stops future generated storefronts being born with it.
Verification
tsc --noEmitreports no errors in the changed file.