Skip wishlist lists fetch for guest visitors to reduce CLS on product listings#464
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
|
Hello @Prestaplugins! This is your first pull request on blockwishlist repository of the PrestaShop project. Thank you, and welcome to this Open Source community! |
There was a problem hiding this comment.
Thanks 🙏 The fix itself is sound, and the strict === false guard is a good defensive default (safe fallback when the def is missing).
Could you regenerate the bundles via npm run build instead of hand-editing them? Two side effects on public/product.bundle.js:
- a UTF-8 BOM (
EF BB BF) got prepended (not present ondev); - the
listsresolver is also compiled into theproductslist/addtowishlist/wishlistcontainerbundles, which don't get the guard. Onlyproduct.bundle.jsis registered on the FO so it works at runtime, but the compiled assets now diverge from the source.
A clean build keeps public/ in sync, drops the BOM, and applies the guard everywhere. Otherwise looks good to me 👍
Replace hand-edited product.bundle.js with webpack output so all bundles embedding the lists resolver stay in sync with _dev/ and no UTF-8 BOM is introduced. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Thanks for the review, @nicohery! I've pushed a follow-up commit that regenerates all public/ assets via npm ci && npm run build, as requested. The lists guard is now compiled into every bundle that embeds the resolver (product, productslist, addtowishlist, wishlistcontainer, button, create, rename). |
nicohery
left a comment
There was a problem hiding this comment.
Thanks — the rebuild resolves my first point.
One more change: the module already knows the login state client-side — Button.vue uses prestashop.customer.is_logged (line 95). The lists resolver could gate on that directly:
lists: async (root, {url}) => {
if (!prestashop.customer.is_logged) {
return [];
}
...
}That lets you drop the new blockwishlistUserLogged JS def and the PHP change entirely, for a single source of truth.
Drop blockwishlistUserLogged JS def and PHP hook change; reuse the same login flag already consumed by Button.vue, per review feedback. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Thanks for the follow-up, @nicohery! Pushed a commit that gates the |
nicohery
left a comment
There was a problem hiding this comment.
Simplification's in. Approving and leaving the rest to other reviewers.
devgetAllWishlistfetch via the Apollolistsresolver when wishlist buttons were initialized on product listing pages. This delayed button rendering and caused Cumulative Layout Shift (CLS).This PR short-circuits the
listsresolver whenprestashop.customer.is_loggedis false, returning an empty array without any network request (same login flag already used byButton.vue).2. Guest: open a category page → Network tab → confirm no
getAllWishlistrequest.3. Guest: wishlist buttons still appear on miniatures; clicking opens login modal as before.
4. Logged-in customer:
getAllWishlistis still called; add/remove from wishlist works on listings and product page.5. Optional: run Lighthouse on a category page and compare CLS before/after.
Summary
listsGraphQL resolver fetch whenprestashop.customer.is_loggedis false.public/bundles vianpm run buildso every bundle embedding the resolver stays in sync with_dev/.Motivation
The
listsquery is only meaningful for authenticated customers. For guests, the server cannot return wishlists, yet the module still performed the HTTP request and waited for the response before stabilizing the wishlist UI on product miniatures.