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

fix: router changes the URL prematurely #2917

Closed
wants to merge 5 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 45 additions & 4 deletions packages/kit/src/runtime/client/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ export class Router {
this.trailing_slash = trailing_slash;
/** Keeps tracks of multiple navigations caused by redirects during rendering */
this.navigating = 0;
/** @type {Array<{ run: () => Promise<void>, abort: () => void }>} */
this.navigation_queue = [];

/** @type {import('./renderer').Renderer} */
this.renderer = renderer;
Expand Down Expand Up @@ -158,12 +160,12 @@ export class Router {
const i2 = location.href.indexOf('#');
const u1 = i1 >= 0 ? url_string.substring(0, i1) : url_string;
const u2 = i2 >= 0 ? location.href.substring(0, i2) : location.href;
history.pushState({}, '', url.href);
// history.pushState({}, '', url.href);
if (u1 === u2) {
window.dispatchEvent(new HashChangeEvent('hashchange'));
}
this._navigate(url, noscroll ? scroll_state() : null, false, [], url.hash);
event.preventDefault();
this._navigate(url, noscroll ? scroll_state() : null, false, [], url.hash);
});

addEventListener('popstate', (event) => {
Expand Down Expand Up @@ -263,6 +265,47 @@ export class Router {
}
this.navigating++;

const self = this;
const cancelable_navigate = () => {
const ctrl = new AbortController();
const { signal } = ctrl;

return {
/**
* @param {URL} url
* @returns {Promise<void>}
*/
run: () => new Promise((resolve, reject) => {
self.renderer.handle_navigation(info, chain, false, { hash, scroll, keepfocus }).then(resolve);
signal.addEventListener('abort', () => {
reject(new DOMException('Promise aborted!'));
});
}),
abort() {
if (!signal.aborted) {
ctrl.abort();
}
}
};
};

// the reason why we have a navigation_queue is
// to cancel the previous navigate action if the user spam the click event
const lastest_cb = this.navigation_queue.shift();
if (lastest_cb) lastest_cb.abort();

const cb = cancelable_navigate();
this.navigation_queue.push(cb);

try {
await cb.run();
} catch (e) {
// if the action aborted, cancel the navigation
return;
}

history.pushState({}, '', url.href);

// remove trailing slashes
if (info.path !== '/') {
const has_trailing_slash = info.path.endsWith('/');
Expand All @@ -279,8 +322,6 @@ export class Router {
}
}

await this.renderer.handle_navigation(info, chain, false, { hash, scroll, keepfocus });

this.navigating--;
if (!this.navigating) {
dispatchEvent(new CustomEvent('sveltekit:navigation-end'));
Expand Down