fix: fulfil beforeNavigate complete when redirected

[teemingc]

fixes #12486

This PR fixes the issue where redirecting causes our internal navigation function to return early and forgets to fulfil the nav.complete promise. We simply need to fulfil the promise after redirecting.

---

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Tests

• Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

• If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

Edits

• Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed.

[changeset-bot]

🦋 Changeset detected

Latest commit: 8ddcbc4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@sveltejs/kit	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[dummdidumm]

While this fixes the issue, I'm wondering if the navigation timing is correct, as it will return before the redirect finishes. There's arguments in various directions:

• it's correct, because we see the redirect as a new navigation, and in fact beforeNavigate is also called again (in that case fulfil should probably be called before _goto)
• the timing is almost correct, but it should reject because the navigation goes elsewhere (in that case reject should probably be called before _goto)
• it's incorrect, because there's a redirect going on. In that case we would somehow need to keep track of the inner nav promise and resolve once that one resolves

[teemingc]

While this fixes the issue, I'm wondering if the navigation timing is correct, as it will return before the redirect finishes. There's arguments in various directions:

• it's correct, because we see the redirect as a new navigation, and in fact beforeNavigate is also called again (in that case fulfil should probably be called before _goto)
• the timing is almost correct, but it should reject because the navigation goes elsewhere (in that case reject should probably be called before _goto)
• it's incorrect, because there's a redirect going on. In that case we would somehow need to keep track of the inner nav promise and resolve once that one resolves

I think 2 is definitely out because redirection doesn't fit the description of "navigation fails or is aborted".

A promise that resolves once the navigation is complete, and rejects if the navigation fails or is aborted. In the case of a willUnload navigation, the promise will never resolve

• https://svelte.dev/docs/kit/@sveltejs-kit#Navigation

Between 1 and 3, I think 1 wins over 3 because beforeNavigate gets called again for the redirection. Unless we change it so that beforeNavigate doesn't re-fire for each redirection? (but that's a breaking change).

[Rich-Harris]

I think we can just fulfil the promise after the redirect has completed. Pushed that change.

It did reveal a type error around _goto, but when I started pulling on the thread it turned out to have far reaching implications. Since it doesn't appear to be causing any tests to fail I slapped a @ts-expect-error TODO on it for now.