Skip to content

fix: resolve Svelte 5.55 navigation state propagation failure#2864

Merged
sydseter merged 3 commits intoOWASP:masterfrom
Mahaboobunnisa123:fix/svelte-5-55-navigation-state
Apr 24, 2026
Merged

fix: resolve Svelte 5.55 navigation state propagation failure#2864
sydseter merged 3 commits intoOWASP:masterfrom
Mahaboobunnisa123:fix/svelte-5-55-navigation-state

Conversation

@Mahaboobunnisa123
Copy link
Copy Markdown
Contributor

@Mahaboobunnisa123 Mahaboobunnisa123 commented Apr 23, 2026

Fixes #2838
The card-browser and mobile layout navigation was broken - clicking links would either skip multiple cards, lose the edition/version/language URL params after a few navigations, or sometimes end up on a 404 page entirely.
After digging into it, the root cause was a combination of things. Svelte 5.55.0–5.55.3 had a regression where reactive state stopped propagating correctly during client-side navigation, which is especially painful with adapter-static since there's no server fallback. Upgrading to 5.55.4 (which patches that regression) fixed the engine side.

Changes applied:

  • Upgraded Svelte to 5.55.4 - resolves the core engine regression
  • cardBrowser.svelte - fixed mouse click firing both onclick and href simultaneously, causing cards to skip multiple steps. Added event.preventDefault() and cleaned up getUrl
  • +layout.svelte - fixed MutationObserver inside $effect never disconnecting on navigation, causing state corruption. Added return () => observer.disconnect() as cleanup
  • Also replaced the legacy svelte/legacy run() + $state pattern in cardFound.svelte, companionCardTaxonomy.svelte, mobileAppCardTaxonomy.svelte, webAppCardTaxonomy.svelte, cards/+page.svelte with proper $derived - these were left over from the Svelte 4→5 migration and were the exact kind of fragile reactive patterns that made the state loss worse.

Testing results:

  • Mouse click navigation moves one card at a time correctly
  • Keyboard arrow navigation works as expected
  • Full URL (edition/version/language params) preserved across all navigations
  • No 404 errors observed
  • No hydration errors in DevTools console
  • pnpm run test - 13 test files, 139 tests, all passed. Coverage: 99.32% statements, 100% functions

Resolved issue: #2838
Screenshots are attached for reference:

Screenshot (838) Screenshot (829) Screenshot (830) Screenshot (828) Screenshot (834) Screenshot (835) Screenshot (832) Screenshot (833)

AI Tool Disclosure

  • My contribution does not include any AI-generated content
  • My contribution includes AI-generated content, as disclosed below:
    • AI Tools: Perplexity AI, Claude AI
    • LLMs and versions: Claude Sonnet 4.6
    • Prompts: Used for guidance on implementation approach(to speedup) and debugging issues[errors]. All the final changes were written, reviewed, and adapted manually.

Affirmation

@sydseter
Copy link
Copy Markdown
Collaborator

@Mahaboobunnisa123 Great! Looks good

@sydseter sydseter marked this pull request as ready for review April 24, 2026 03:20
Copilot AI review requested due to automatic review settings April 24, 2026 03:20
@sydseter sydseter requested review from cw-owasp and rewtd as code owners April 24, 2026 03:20
@sydseter sydseter merged commit b5ec0d4 into OWASP:master Apr 24, 2026
9 of 10 checks passed
@sydseter
Copy link
Copy Markdown
Collaborator

@Mahaboobunnisa123 Thank you for your help.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a SvelteKit (adapter-static) navigation/state propagation regression affecting card browsing and mobile layout behavior by upgrading Svelte and adjusting client-side navigation/reactivity patterns.

Changes:

  • Upgraded svelte to 5.55.4 (and lockfile updates) to pick up the upstream fix.
  • Updated card navigation handling (cardBrowser.svelte) to prevent double-navigation on click and rely on goto() + hrefs consistently.
  • Reworked several components away from svelte/legacy run() and manual $state updates toward $derived-based reactivity; added cleanup for a MutationObserver in the root layout.

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
cornucopia.owasp.org/src/routes/cards/+page.svelte Updates card/mapping state derivation for the cards index page.
cornucopia.owasp.org/src/routes/+layout.svelte Ensures MutationObserver is disconnected on navigation (avoids leaking observers/state).
cornucopia.owasp.org/src/lib/components/cardBrowser.svelte Prevents click navigation from firing both default anchor navigation and JS navigation.
cornucopia.owasp.org/src/lib/components/cardFound.svelte Adjusts component props usage and switches mapping/ASVS derivations to $derived.
cornucopia.owasp.org/src/lib/components/webAppCardTaxonomy.svelte Removes svelte/legacy usage; derives mappings/attacks reactively.
cornucopia.owasp.org/src/lib/components/mobileAppCardTaxonomy.svelte Removes svelte/legacy usage; derives mappings/attacks reactively.
cornucopia.owasp.org/src/lib/components/companionCardTaxonomy.svelte Removes svelte/legacy usage; derives mappings/attacks reactively.
cornucopia.owasp.org/package.json Bumps Svelte dependency version.
cornucopia.owasp.org/pnpm-lock.yaml Lockfile updates for the Svelte upgrade and transitive deps.
Files not reviewed (1)
  • cornucopia.owasp.org/pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)

cornucopia.owasp.org/src/routes/+layout.svelte:36

  • The $effect now checks if (document) and the file still imports browser without using it. Consider switching this guard to if (browser) (or typeof document !== 'undefined') and dropping the unused import, so the intent is clear and the code won’t risk document access in non-browser contexts.
    // add styles back in non-CSP violating way
    $effect(() => {
    if (document) {
        const observer = new MutationObserver((mutations) => {
        for (const mutation of mutations) {

Comment on lines +41 to +43
let mappings = $derived(controller.getCardMappings(card.id));
let attacks: Attack[] = $derived(GetCardAttacks(card.id));
const asvsVersion = $derived(card.version < '3.0' ? '4.0.3' : '5.0');
Comment on lines +36 to +41
const KEYCODE_RIGHT = 39;
const KEYCODE_LEFT = 37;
const nextCard = cards.get(card.next);
const prevCard = cards.get(card.prevous);
if (event.keyCode == KEYCODE_RIGHT && nextCard) goto(getUrl(nextCard));
if (event.keyCode == KEYCODE_LEFT && prevCard) goto(getUrl(prevCard));
Comment on lines +44 to +46
let card: Card = $state(undefined as unknown as Card);
let mapping: any = $state([]);

Comment on lines +42 to +62
let version: string = $state(VERSION_WEBAPP);
let suit: string = $state('');
let card: Card = $state(undefined as unknown as Card);
let mapping: any = $state([]);

$effect(() => {
const c = cards;
const v = version;
if (!c) return;
if (v === VERSION_WEBAPP) card = c.get('VE2') as Card;
if (v === VERSION_MOBILEAPP) card = c.get('PC2') as Card;
if (v === VERSION_COMPANION) card = c.get('AAI2') as Card;
});

$effect(() => {
const c = card;
const md = mappingData;
const v = version;
if (!c || !md) return;
mapping = new MappingController(md.get(v)).getCardMappings(c.id);
});
@Mahaboobunnisa123
Copy link
Copy Markdown
Contributor Author

@Mahaboobunnisa123 Great! Looks good

Thank you for your kind words.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update to latest version of Svelte

3 participants