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

onNavigate lifecycle function #560

Closed
Rich-Harris opened this issue Mar 17, 2021 · 9 comments · Fixed by #3293
Closed

onNavigate lifecycle function #560

Rich-Harris opened this issue Mar 17, 2021 · 9 comments · Fixed by #3293
Labels
p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc. router
Milestone

Comments

@Rich-Harris
Copy link
Member

#552 suggested a need for a lifecycle function that gets called when navigation occurs but the destination component is the same as the one we're already on (since onMount only runs the first time the component is mounted, and the alternative — unmounting components upon navigation — is bad for a variety of reasons).

I propose an onNavigate hook that lives in $app/navigation and looks something like this:

export function onNavigate(fn) {
  let mounted = false;

  const unsubscribe = getStores().page.subscribe(() => {
    if (mounted) fn();
  });

  onMount(() => {
    mounted = true;
    fn();

    return () => {
      unsubscribe();
      mounted = false;
    }
  });
}
<script>
  import { onMount } from 'svelte';
  import { onNavigate } from '$app/navigation';

  onMount(() => {
    // runs once when the page mounts
  });

  onNavigate(() => {
    // runs when the page mounts, and also whenever SvelteKit
    // navigates to a new URL but stays on this component
  });
</script>

As mentioned in #404 (comment):

There might be some nuance around ensuring that the callback doesn't run immediately before the component is unmounted, but you get the gist. This would work in any component (layout or otherwise) like a normal lifecycle function, including components created after the page was initially created (e.g. lazily-loaded stuff).

@lowi
Copy link

lowi commented Mar 26, 2021

We need a hook onBeforeNavigate as well. As referenced by @benmccann in sapper#1047, it's very useful to be able to cancel navigation when a user is editing something and has forgotten to click "Save".
For example, I show a dialog "You didn't save your changes. Do you want to stay on the page or lose your changes?".
A basic onNavigate hook doesn't allow this kind of behavior.

@PatrickG
Copy link
Member

We need a hook onBeforeNavigate as well. As referenced by @benmccann in sapper#1047, it's very useful to be able to cancel navigation when a user is editing something and has forgotten to click "Save".
For example, I show a dialog "You didn't save your changes. Do you want to stay on the page or lose your changes?".
A basic onNavigate hook doesn't allow this kind of behavior.

If this will not be handled by svelte/kit, I will provide a package for it, simliar to this.
But I think it would be good, if this were handled by svelte/kit.
I would call it beforeNavigate analogous to beforeUpdate.

One question is, should onbeforeunload handled by it, too?

@lowi
Copy link

lowi commented Mar 26, 2021

@PatrickG It would make sense to me if the router check if window.onbeforeunload is defined and call it to know if it can change route: that was my first attempt with sapper before searching and using your sapper's PR.

@PatrickG
Copy link
Member

PatrickG commented Mar 26, 2021

@PatrickG It would make sense to me if the router check if window.onbeforeunload is defined and call it to know if it can change route: that was my first attempt with sapper before searching and using your sapper's PR.

I didn't mean it that way.
It should be no problem to do this in src/runtime/client/router.js, this runs only on client anyways.

I mean, in sapper-navigation-enhancer, I have this:

beforeNavigate(callback: () => any, useBeforeUnload: boolean): () => void;

When useBeforeUnload (bad variable naming?) is true, a onbeforeunload listener will be created additionally.
But I don't know whether svelte/kit should even offer the onbeforeunload functionality.

Btw, maybe we should create another issue for this. Because it is not onNavigate.

@lukasIO
Copy link
Contributor

lukasIO commented Apr 10, 2021

Another benefit of an onBeforeNavigate lifecycle function is that it would allow for a return value that could get added to the history state.
That would make it possible to do things like

onBeforeNavigate() {
  return { previousUrl: location.href }
}

and access this object on the next route with history.state.previousUrl in a pretty concise way.

edit: just realized by looking at the source, that returning the state should not be needed as the current state gets already injected in the router 🎉
the usecase of the lifecycle function for pushing stuff to the history state would still be valid imo, even if it's just

onBeforeNavigate() {
  history.state = { ...history.state, previousUrl: location.href }
}

@Rich-Harris Rich-Harris modified the milestones: 1.0, post-1.0 May 1, 2021
@benmccann benmccann added the p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc. label Aug 5, 2021
@TeemuKoivisto
Copy link

Hey has there been any new progress on this? I just now wanted to add this to my app but as it seems not possible, I resulted to making this very ugly hack:

https://gist.github.com/TeemuKoivisto/a22ba96d4eb52b95776202d5038815b7

Which seems to prevent most cases. Well, I couldn't bother fixing that forward backspacing but I just want to move on to another things. Good enough for me I guess. And well the onclicks on those links could be intercepted better.

@zommerberg
Copy link
Contributor

zommerberg commented Dec 2, 2021

Additionally, It would be nice if the onBeforeNavigate adds the ability to intercept a navigation intent and cancel it if needed for features like "Confirm that you want to leave" modals.

I've created a separate request for the onBeforeNavigate feature.
More information here: #2974

@HenrijsS
Copy link

Don't know about the onBeforeNavigate, but I just made a simple computed function for onNavigation like this:

import { page } from "$app/stores";

let pageLog: string = $page.path;

$: if (pageLog !== $page.path) {
  pageLog = $page.path;
  // On navigation logic goes here
}

This checks if the URL has changed and if yes, executes a function.

@TeemuKoivisto Maybe this works cleaner for you.

P.S. I absolutely love $: as it's so powerful <3

@bluwy
Copy link
Member

bluwy commented Dec 24, 2021

Another syntax idea after skimming #2982. It would work very similiarly to vue router:

onNavigate(({ from, to, next }) => {
  // before route
  next() // confirm route
  // after route
})

next(false) to cancel navigation. I don't think we need to implement next('/') as that's the same as goto('/'). This way we can have before, after hooks all in one function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc. router
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants