Skip to content

fix(shop): repair duplicated initialization block that left shop.js unparsable - #1701

Open
MOHITKOURAV01 wants to merge 1 commit into
AnthropicBots:mainfrom
MOHITKOURAV01:fix/1696-shop-init-parse-error
Open

fix(shop): repair duplicated initialization block that left shop.js unparsable#1701
MOHITKOURAV01 wants to merge 1 commit into
AnthropicBots:mainfrom
MOHITKOURAV01:fix/1696-shop-init-parse-error

Conversation

@MOHITKOURAV01

@MOHITKOURAV01 MOHITKOURAV01 commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Closes #1696

What was wrong

frontend/scripts/shop.js did not parse. npm run check:syntax was red on main:

❌ 1 of 652 JavaScript file(s) failed to parse:
  frontend/scripts/shop.js:2499
      Unexpected end of input

The responsive-design refactor duplicated the initialization block at the bottom of the file and then interleaved the two copies. The tail ended up with two DOMContentLoaded registrations: the first with an arrow body that is never closed and a clear-filters listener that lost its body and picked up { rootMargin: "200px 0px" } — an IntersectionObserver options object belonging to observeSentinel() — as its third argument; the second re-doing the same element lookups, stopping halfway through them, and running into orphaned fragments of the first handler's body that referenced a filterUrlParams no longer in scope.

A SyntaxError aborts the whole script, so shop.html had no JavaScript: no product fetch, no filters, no sort, no infinite scroll, no search suggestions.

What this does

Collapses the two blocks back into the single handler the page has always needed, reconstructed from b8b934a^, and restores the clear-filters behaviour the merge stranded. That behaviour matters beyond tidiness: a shopper arriving from the mega menu carries category/subcategory in the query string, and filters.megaCategory / filters.megaSubcategory are seeded from them. Resetting only the checkboxes leaves both in place, so the next applyFilters() re-applies the filter the button just cleared and a reload brings it back. The handler now strips the params, rewrites the address bar, clears the mega filters, and re-runs the query.

Two small things kept from the reconstruction rather than the original:

  • handleCategorySelect returns early when a card has no data-category, instead of building a checkbox with value="undefined".
  • The stray addEventListener options object is gone. The browser silently accepts it — it reads as an options bag with no recognised keys — which is why this only ever looked like a listener that was wired and was not.

The call shape

The listener is written as the multi-line form:

document.addEventListener(
    "DOMContentLoaded",
    () => {
        ...
    }
);

not the compact document.addEventListener("DOMContentLoaded", () => { ... });. That is not a style preference — tests/mergeScarRegression.test.js already pins it:

it('closes that listener rather than running on into what follows', () => {
    // The #1444 scar itself: `}\n);` closes the arrow function and the call
    // it is an argument to. Dropping the `);` merged the listener into the
    // next declaration.
    expect(source.slice(listener)).toMatch(/\n\s*\}\n\s*\);/);
});

That suite has been red on main since the refactor. This makes its parses, has exactly one DOMContentLoaded initialiser and closes that listener cases green again.

Guard

backend/tests/frontendScriptParse.test.js compiles every file under frontend/scripts with vm.Script, the same parse goal a classic <script> tag and CommonJS use, retrying genuine ES modules the way scripts/check-syntax.js does. It then pins the shape of shop.js's init block: one DOMContentLoaded registration, one declaration of filterUrlParams with no stranded uses, no observer options handed to addEventListener, and all six steps of the clear-filters handler present.

check:syntax already catches the parse failure, but it is a separate CI step; having it in the jest suite puts the failure next to the behaviour it breaks.

Verification

$ npm run check:syntax
✅ syntax check passed — 652 JavaScript file(s) parsed cleanly

$ backend/node_modules/.bin/jest --rootDir backend tests/frontendScriptParse.test.js
Tests:       81 passed, 81 total

Confirmed the guard is real by stashing the shop.js fix and re-running the new test against the broken file:

✕ shop.js parses as a classic script
✕ exactly one DOMContentLoaded listener is registered
✕ no statement references an undeclared filterUrlParams
✕ addEventListener is never handed IntersectionObserver options
Tests:       4 failed, 77 passed, 81 total

The pre-existing tests/mergeScarRegression.test.js goes from three failures on main to one:

frontend/scripts/shop.js
  ✓ parses
  ✓ has exactly one DOMContentLoaded initialiser
  ✓ closes that listener rather than running on into what follows
  ✓ declares no function twice
  ✓ declares the functions its own code calls
  ✓ resolves the clear-filters button and sort select by the ids the page uses
  ✓ has one owner for the clear-filters click

The one it still reports — frontend/scripts/product-cards-home.js › declares the stock bindings its card template reads — is a different file and a separate defect from the same refactor, and is out of scope here.

Also ran check:assets, check:a11y, check:sitemap, check:boot and check:modules — all pass.


CI note

Syntax check ✅ and Server boots ✅ — the two jobs that were red on main because of this file.

Backend tests is still red, on suites this PR does not touch. All five already fail on main:

suite failing case fixed by
mergeScarRegression product-cards-home.js › declares the stock bindings its card template reads not yet — separate defect from the same refactor
cartBulkSelection does not put handlers on the global object not yet
wishlistReads 7 cases, all p.is_active = 1 missing from the queries not yet
migrationViewColumns no two migrations claim the same version #1705
fraudMonitoringQueue takes the next free migration number #1705

This PR takes mergeScarRegression's frontend/scripts/shop.js block from three failures to zero:

frontend/scripts/shop.js
  ✓ parses
  ✓ has exactly one DOMContentLoaded initialiser
  ✓ closes that listener rather than running on into what follows
  ✓ declares no function twice
  ✓ declares the functions its own code calls
  ✓ resolves the clear-filters button and sort select by the ids the page uses
  ✓ has one owner for the clear-filters click

The Vercel check fails on every PR in this repository with "Authorization required to deploy" against the bhuvanshs-projects team, unrelated to any change.

@hydra-maintainer

Copy link
Copy Markdown

🔍 Quality Gate Report

✅ All quality gates passed!

Status Check Details
Linked Issue PR description references a closing issue ✅

@vercel

vercel Bot commented Aug 26, 2026

Copy link
Copy Markdown

Someone is attempting to deploy a commit to the Bhuvansh's projects Team on Vercel.

A member of the Team first needs to authorize it.

@hydra-maintainer

Copy link
Copy Markdown

🤖 AI Code Review

🔴 Score: 50/100 | comment

AI review unavailable at this time.


Automated AI review — a human maintainer will also review.

@hydra-maintainer

Copy link
Copy Markdown

💡 Suggested reviewers based on relevant file history: @Aditya8369, @Pcmhacker-hero

@hydra-maintainer

Copy link
Copy Markdown

🟡 PR Health Score: 55/100

This PR's health score is below the 75/100 threshold for a healthy label.

Status Signal
Test coverage
Linked issue
PR description (≥50 chars)
DCO sign-off
⬜ 0 approval(s) Approvals
Diff size < 400 lines

Improving these signals will help reviewers engage faster and raise your score. 💪

…nparsable

The responsive refactor duplicated the tail of frontend/scripts/shop.js and
interleaved the two copies. The result did not parse -- an unclosed arrow body,
a second half-finished DOMContentLoaded registration, statements referring to a
filterUrlParams no longer in scope, and an IntersectionObserver options object
passed as the third argument to addEventListener. The browser answered with
'Unexpected end of input' and ran nothing at all on shop.html.

Collapse the two back into the one handler the page needs, and restore the
clear-filters behaviour the merge stranded: strip category/subcategory from the
query string, rewrite the address bar, clear filters.megaCategory and
filters.megaSubcategory, then re-run applyFilters.

Add backend/tests/frontendScriptParse.test.js, which compiles every file under
frontend/scripts with the script goal a <script> tag uses, and pins the shape of
shop.js's initialization block so the duplication cannot come back green.
@MOHITKOURAV01
MOHITKOURAV01 force-pushed the fix/1696-shop-init-parse-error branch from 37e1dd5 to faa44f6 Compare August 26, 2026 16:22
@hydra-maintainer

Copy link
Copy Markdown

🔍 Quality Gate Report

✅ All quality gates passed!

Status Check Details
Linked Issue PR description references a closing issue ✅

@hydra-maintainer

Copy link
Copy Markdown

🤖 AI Code Review

🔴 Score: 50/100 | comment

AI review unavailable at this time.


Automated AI review — a human maintainer will also review.

@hydra-maintainer

Copy link
Copy Markdown

💡 Suggested reviewers based on relevant file history: @Aditya8369, @Pcmhacker-hero

@hydra-maintainer

Copy link
Copy Markdown

🟡 PR Health Score: 55/100

This PR's health score is below the 75/100 threshold for a healthy label.

Status Signal
Test coverage
Linked issue
PR description (≥50 chars)
DCO sign-off
⬜ 0 approval(s) Approvals
Diff size < 400 lines

Improving these signals will help reviewers engage faster and raise your score. 💪

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] frontend/scripts/shop.js fails to parse — shop page JavaScript is entirely dead and check:syntax is red on main

1 participant