Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

afterNavigate has incorrect type and missing from if using browser nav following an action which throws a redirect #8666

Closed
fzembow opened this issue Jan 23, 2023 · 3 comments

Comments

@fzembow
Copy link

fzembow commented Jan 23, 2023

Describe the bug

In the case that a form action throws a redirect(), it seems that using the browser back navigation will not correctly set the AfterNavigation object passed to the callback of afterNavigate.

Normally, browser back navigation shows popstate as the type, like this:

{
  "from": {
    "params": {},
    "route": {
      "id": "/other-page"
    },
    "url": "http://127.0.0.1:5173/other-page"
  },
  "to": {
    "params": {},
    "route": {
      "id": "/"
    },
    "url": "http://127.0.0.1:5173/"
  },
  "willUnload": false,
  "type": "popstate",
  "delta": -1
}

However, if I submit to a form action, get redirected, and then use the browser back button, it just looks like an initial load:

{
  "from": null,
  "to": {
    "params": {},
    "route": {
      "id": "/"
    },
    "url": "http://127.0.0.1:5173/"
  },
  "willUnload": false,
  "type": "enter"
}

I would expect from to be set here and type to be "popstate", like in the first example.

Reproduction

See https://github.com/fzembow/sveltekit-after-navigation-bug

STEPS

  1. yarn dev
  2. Submit the form on the main page and get redirected to /other-page
  3. Click back in your browser

RESULT

Observe that the navigation type is "enter" and not "popstate", and that from is null.

EXPECTED

I would expect the navigation type to be "popstate" and for "from" to be set.


This is the first page

/routes/+page.svelte

<script lang="ts">
	import { afterNavigate } from '$app/navigation';
	import type { AfterNavigate } from '@sveltejs/kit';

	let navigation: AfterNavigate;
	afterNavigate((_navigation) => {
		navigation = _navigation;
	});
</script>

<form method="POST">
	<input type="submit" />Submit the form and get redirected
</form>

<h1>Navigation</h1>
{#if navigation}
	<b>Type: {navigation.type}</b>
	<pre><code>
		{JSON.stringify(navigation, null, 2)}
	</code></pre>
{/if}

This is that page's actions:
/routes/+page.server.ts

import { redirect, type Actions } from '@sveltejs/kit';

export const actions: Actions = {
	default: async () => {
		throw redirect(302, `/other-page`);
	}
};

And this is the target page (doesn't really matter what the content is here since we will just use the browser back)
/routes/other-page/+page.svelte

<h1>This is the other page</h1>

Logs

No response

System Info

System:
    OS: macOS 12.6
    CPU: (10) arm64 Apple M1 Max
    Memory: 21.09 GB / 64.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 16.15.0 - ~/.volta/tools/image/node/16.15.0/bin/node
    Yarn: 1.22.17 - ~/.volta/tools/image/yarn/1.22.17/bin/yarn
    npm: 8.5.5 - ~/.volta/tools/image/npm/8.5.5/bin/npm
  Browsers:
    Chrome: 109.0.5414.87
    Firefox: 108.0.2
    Safari: 16.0
  npmPackages:
    @sveltejs/adapter-auto: ^1.0.0 => 1.0.2 
    @sveltejs/kit: ^1.0.0 => 1.2.2 
    svelte: ^3.54.0 => 3.55.1 
    vite: ^4.0.0 => 4.0.4 


### Severity

serious, but I can work around it

### Additional Information

_No response_
@eltigerchino
Copy link
Member

I believe this issue occurs because your form submission does a full page reload, causing the client side router to re-initialise on the redirected page, and also when you press back.

You’ll need to enhance your form to retain the client-side routing history.

https://kit.svelte.dev/docs/form-actions#progressive-enhancement-use-enhance

@fzembow
Copy link
Author

fzembow commented Jan 23, 2023

Oh yeah - I did notice that it worked if enhance were set. I was a bit worried about the behavior being different for non-javascript enabled users, although I guess that is somewhat of an academic concern.


I am curious whether in this situation, back navigation could be inferred that the navigation was a browser navigation using the back/forward cache. I'm pretty sure bfcache would be correct in the case.

However, I don't think pageshow actually works at all in svelte kit (although it seems to have been added in the past). For example, the following never alerts on any page:

<svelte:window
	on:pageshow={(e) => {
		alert('pageshow');
	}}
/>

... although of course I have no idea how complex adding an event listener for pageshow on the window object would be, and that would be a separate issue probably.


What do you think? I'm fine just using enhance myself, but I figured I'd ask.

@dummdidumm
Copy link
Member

The pageshow event probably isn't fired because it's listened to too late.
Infering this from back/forward cache is unreliable and could be confusing/breaking expectations of behavior elsewhere.
Closing since the issue is solved through use:enhance

@dummdidumm dummdidumm closed this as not planned Won't fix, can't repro, duplicate, stale Jan 25, 2023
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

No branches or pull requests

3 participants