Before and after hooks for routing about kit HOT 71 CLOSED

After trying to find a reasonable way around this now, I'm 99% erring on this behaviour being a bug / broken feature.

Not having the ability to use onMount and onDestroy means you have to write your own change logic for every component and sub-component which sits underneath your top-level component.

Listening for the $page store to change is fine if you're only one component deep, but the amount of subscribing/reactive declarations you need to get everything in the hierarchy to update just becomes unmaintainable very quickly.

Being able to rely on components mounting and un-mounting is the only sensible way I can think here of restoring state, since if you think about it, a route change is akin to "tearing down" the page component and "mounting" a new page component.

from kit.

IMO this is 100% a bug and the fact that it accidentally has some benefits is irrelevant. Let me try to convince you all:

The problem is that the developer now has to consider not just where a user is but how she got there (or where she is coming from) to reason about their app. Consider @antony's example where there are 3 views:

https://example.com/blog/fish/how-to-feed-a-fish
https://example.com/blog/fish/how-to-eat-a-fish
https://example.com/blog/horse/how-to-feed-a-fish-to-a-horse

And the dynamic params at the top level:

/blog/fish/[title].svelte
/blog/horse/[title].svelte

This example is very plausible if it is e.g. team and user instead of fish and horse. If the user navigates from horse 1 to fish 1, she will be in one state (new content loaded). If a user navigates from fish 1 to fish 2 directly, she will be in a different/inconsistent state (no new content loaded). But if the user who got to fish 2 via fish 1 refreshes her browser, she'll see different content.

Consider these two functions:

function navigateToFish(title) {
   goto(`/blog/fish/${title}`);
}

function navigateToFish2(title) {
   window.location = `/blog/fish/${title}`;
}

I'd argue that they should behave exactly the same way. The user is navigating to the same view; the developer took the recommendation to use Sapper's goto when handling navigation. But if the developer uses the first function on any fish pages, the two functions will not behave the same way.

IMO the URL represents where the user is regardless of how they got there, and Sapper should default to idempotence: a request produces the same result every time.

To address the examples of a photo album or ecommerce site, I see those as performance optimizations. You presumably know where the user is coming from (another photo or another item of a certain type) and going to, so you want to optimize the transition and possibly add an animation. Sure! Why not just make {remount: true} the default and if you happen to know you're going from one photo to another, you can optimize by passing {remount: false}?

from kit.

This needs to be documented immediately.

I can't tell you how many hours my team and I dumped into this problem.

If it weren't for finding this issue and combing through it, we would have thought Sapper was completely broken.

It might be the reason why teams like IBM's Carbon Design team stopped supporting Svelte/Sapper.

I propose something akin to the following be added to the navigation section of the Sapper docs:

If you're wondering why the hell your blog articles or other collection pages aren't loading data correctly between client-side page navigations, you need to read this.

By design, Svelte doesn't remount a component if it is being reused. This allows Svelte to remain blazing fast, because no constructor or onMount/onDestroy methods have to be performed during navigation.

This has implications for client-side navigation in Sapper: when navigating between pages of the same subpath (i.e. between blog articles) in a component that relies on external API calls or other asynchronous data, the component won't be hydrated with data. Or, in terms of Sapper's filename ecodings, when traveling navigating from blog/[slug].svelte to another instance of blog/[slug].svelte, data won't appear to be flowing into the component correctly. Examples of this include clicking on a Featured Article link that appears on a blog article, or navigating between adjacent blog articles.

Not to fear, this is by design. There is a way to handle it.

To handle it, subscribe to the $page svelte store. Detect when navigation occurs. Use Sapper's goto function to programatically perform the navigation. This triggers Sapper to hydrate the component with the data we expect from the preload function.

For example:

//<!-- [slug.svelte] -->
<script context="module">
	function preload (...) { 
		/*
		  Perform external API calls, whatever (i.e. Contentful, Memberstack, etc.)
		*/
		return articleData;
	}
</script>

<script>
  import { onDestroy } from "svelte"
  import { stores, goto } from "@sapper/app";
  export let articleData;

  let { page } = stores();
  let unsubscribeToPageStore = page.subscribe(async (pageData) => {
			// HYDRATION!
      goto(pageData.host+pageData.path)
  })
  onDestroy(() => {
		unsubscribeToPageStore()
  })
</script>

I see the power and usefulness of Sapper/Svelte and this could be causing a lot of pain for a lot of people. This information needs to be front-end center in the navigation section of the Sapper API docs

Also, the method above may or may not create an infinite loop of browser navigation... basically Sapper is broken unless you bail by forcing rel=external, therefore completely defeating the benefits of Sapper's client-side navigation.

from kit.

I like sapper-remount 👍

from kit.

as does Rich (in sveltejs/sapper#295 he says "I would argue that destroy/recreate should be the default behaviour")

"A foolish consistency is the hobgoblin of little minds" — Emerson

I very vaguely recall some conversations about this issue as we switched to <slot>-based layouts way back when, and FWIW the current behaviour definitely was deliberate. The thinking was that aside from being more efficient (since you're not recreating a bunch of stuff only some of which, possibly very little, has changed), it makes it much easier to create appealing page transitions when you're navigating between different pages belonging to the same route:

http://aboard-parent.surge.sh/conditions/green

It's true that we haven't really capitalised on this ability, or done a good job of communicating it. But that's my argument in favour of keeping things as they are. It does make some things a bit harder, but in return it makes other things possible.

from kit.

Just coming back to this issue since my use-case has required changes to the way I do this.

I'd solved for the original case with some complex afterUpdate logic and memoisation, but now I've moved the inner component to a fully store+context based model, it isn't reactive in the way it used to be. I'm now 2-3 hours deep re-solving the same problem.

No matter what sort of optimal rendering scenario we're gaining here, the developer experience is extremely poor. I think onEnter or onExit (with whatever naming is preferred) would be a great solution. The PR at sveltejs/sapper#1047 might make a nice starter.

If a PR like this were to exist, would there be any resistance?

from kit.

The component's not re-mounting though. You can listen to changes to the $page store with reactive statements, which is how I think you're supposed to deal with this. I don't think this is a bug and I don't think Sapper should start destroying the old page component and creating a new one when switching between pages that fall under the same route.

from kit.

This seems very odd to me. If my page preloads content, say I'm writing a blog, and that blog has links to related content:

https://example.com/blog/fish/how-to-feed-a-fish
https://example.com/blog/fish/how-to-eat-a-fish
https://example.com/blog/horse/how-to-feed-a-fish-to-a-horse

If my categories are hardcoded:

/blog/fish/[title].svelte
/blog/horse/[title].svelte

If I were to click on a link to a related blog about fish, I'd end up not calling onMount (and therefore not loading the content for the new blog)

If I were to click on a link to a related blog about horses, I'd end up calling onMount (and therefore loading the content for the new blog)

This feels like a seemingly innocuous set of links that do radically different things, and exhibit radically different behaviours, despite appearing identical to both the end user and the developer.

I've added the tag docs to this, but I'm not sure how I'd document it. What would be the rules regarding whether switching route does or doesn't require a subscription to the page store in order to load it's content?

I think this behaviour is mysterious, and (at least for us) has caused quite a serious problem in that pricing for one item remains on the page when you click a new item, since it wasn't obvious that I would need to listen to the page store in order to notify the component that a new page had loaded. It seems like a bit of a gotcha.

from kit.

Just to add a couple more use cases on the side of keeping the current default behaviour, that I've come across in a site I'm building with sapper:

• Audio - it would be really confusing if the default behaviour was to tear down a component for audio and anything else with state external to svelte (I appreciate this could be solved with layout components, but it still seems like a bit of an unexpected gotcha for default behaviour - as has already been mentioned, I think, people don't always read docs)
• Canvas - I'm using onMount to initialise a canvas for audio visualisations, and that again would've caught me out if it was torn down on a page transition (this again could be partially solved with layout, I suppose, but it probably wouldn't be a very clean solution, and may not work in all cases)
• Essentially anything with dynamic cross-route state, canvas and audio being just two examples. As an application framework, I'd argue that this should be the use case that sapper targets, rather than page based things.
• Consistency with other solutions - which maybe isn't (and probably shouldn't be) a goal of sapper. Regardless, the current default is how most other things deal with it (I'm thinking of vue router specifically, but others also apply), and it would be pretty unexpected to have different behaviour in sapper

from kit.

from kit.

Given how many people find this behavior counterintuitive and the decision to keep the behavior it'd probably be a good idea to add an entry to the FAQ explaining why it happens and some workaround recommendation (or a link to this issue, which has a few workarounds).

from kit.

I'd be happy with this suggestion, as long as there was some way to make the onMount and onDestroy hooks get called. I can understand the reason behind wanting to be efficient with creating/destroying components.

However I will maintain, and with good reason, that this behaviour is a gotcha and is not obvious. It really doesn't make sense to me that I would have to know an intimate, internal rendering concern of Sapper to determine whether certain parts of my page's content will end up with updated state or not, depending whether the route I came from is the same, or a different physical component. It's doesn't follow Principle Of Least Astonishment. For this reason I'd suggest that this feature/rendering optimisation/gotcha is documented quite clearly.

Side-note: Absolutely regretting your gif.

from kit.

we are in 2021.
Isn't this big problem solvable?

from kit.

I feel like part of the problem here is that onMount probably isn't the right place to be loading blog content. If you load it in preload, that will get re-run even on navigation to the same page component, and all the props will get their values updated. onMount also prevents the content from being SSR'd.

from kit.

A probable reason that the reactive statements did not help here is that the reactive statements themselves do not reference the changing properties.

Aha! OK, thanks.

You can fix that by doing something like $: fetchLessonData(lessonData[keyboard]) instead.

Fantastic! This actually works. Thank you.

Solution:

  export let keyboard = $keyboardLayout;
  export let lessonGroup = $currentLessonGroup;
  export let lesson = $currentLesson;

  $: lessonCharacters = fetchLessonData(keyboard, lessonGroup, lesson);
  $: promptLines = generatePromptLines(keyboard, lessonGroup, lesson);

  function fetchLessonData(keyboard, lessonGroup, lesson) {
    return lessonData[keyboard]["rowLessonGroups"]
      .filter(g => g.metaData.slug === lessonGroup)[0].lessons
      .filter(l => l.metaData.slug === lesson)[0].characters
  }

  function generatePromptLines(keyboard, lessonGroup, lesson) {
    return [...Array(3)].map(x => generateSinglePromptLine());
  }

I now have a function that requires 3 arguments that aren't used, but I can fix that like this:

  let lessonCharacters;
  let promptLines;

  $: initialise(keyboard, lessonGroup, lesson);

  function initialise(keyboard, lessonGroup, lesson) {
    fetchLessonData(keyboard, lessonGroup, lesson);
    generatePromptLines();
  }

  function fetchLessonData(keyboard, lessonGroup, lesson) {
    lessonCharacters = lessonData[keyboard]["rowLessonGroups"]
      .filter(g => g.metaData.slug === lessonGroup)[0].lessons
      .filter(l => l.metaData.slug === lesson)[0].characters
  }

  function generatePromptLines() {
    promptLines = [...Array(3)].map(x => generateSinglePromptLine());
  }

EDIT: Adding this for anyone looking for answers.. it turns out there's a simpler solution to this problem which is to use the new {#key expression} https://svelte.dev/docs#key at the top level component which will trigger the child components to rerender.

    {#key [keyboard, lessonGroup, lesson]}
      <TypingPrompt
        bind:keyboard
        bind:lessonGroup
        bind:lesson
      />
    {/key}

(TypingPrompt is the component that previously contained the above $: initalise function. keyboard, lessonGroup and lesson are updated by Sapper's preload function when the route changes, and the {#key [keyboard, lessonGroup, lesson]} statement reacts to these values changing (when the route changes) and rerenders TypingPrompt prompting a rerender of the components all the way down the tree.

from kit.

I'm trying to work around this using the page store, and the problem is becoming clearer - I don't believe the current behaviour makes any sense. I have a shared component on my page which builds a bunch of stores and puts them in the context.

Setting the context can only be done on initialisation, and if the component doesn't re-initialise (since it's not re-mounted between route-changes), I can't change the items inside the context.

If I'm loading in content and passing in a store to a component, then I don't think there is a reasonable way to switch-out that store, which leaves me in a position that I'm unable to change that component's content.

I've updated the repro app to use the store. The logic seems solid the app straight-forward, but the behaviour just seems wrong. There might be a way to fix it by tying to the page store (which I will attempt next, but if there is, this really doesn't seem like obvious behaviour)

from kit.

it's going to trip people up regardless, because people are bad at reading docs (counting myself here). However I have less sympathy for people who trip because they haven't RTFM'd.

I am still strongly in favour of adding it as an attribute as suggested. Is sapper- the best way or is sapper: a more consistent prefix? Is sapper:prefetch = rel="prefetch" reasonable?

Either way, I think remount is descriptive enough, so something like:

sapper:remount or sapper-remount

Edit: I've just realised, sapper: won't work as this is part of the Svelte API right? So sapper-remount perhaps.

from kit.

I like the idea of enter/exit or load/unload. But IMO Sapper can't ignore onMount.

Here's where I'm coming from: I see Sapper as a wrapper around Svelte. It shouldn't change any of the contracts Svelte sets out; it should only add functionality.

The docs for Svelte say onMount should be used for lazy loaded data that won't be rendered server-side:

It's recommended to put the fetch in onMount rather than at the top level of the <script> because of server-side rendering (SSR). With the exception of onDestroy, lifecycle functions don't run during SSR, which means we can avoid fetching data that should be loaded lazily once the component has been mounted in the DOM.

So say I have a bunch of Svelte components that load data using onMount. Say I was previously using something else, and I pick up Sapper and start using it. Sapper has the responsibility to honor all of the guarantees that Svelte put forth, including running onMount when a component is loaded. Even if under the hood Sapper isn't really loading the component because it's smart enough to know that its really dealing with two dynamic routes of the same type, that doesn't really matter. That's Sapper internals, and Sapper shouldn't be built so that developers have to know that stuff to get it to work right.

from kit.

I just got stung by this. I converted my plain svelte app (a typing trainer for learning to type Hangul characters - never miss a backlink opportunity!) to a Routify app and found that my nav links were updating the URL but not causing the main component to update the DOM, even though I could see in the debugger that the props were actually updating.

Initially I assumed this was a Routify issue, after a good few hours trawling through Routify docs, source code and stepping through functions in the Chrome debugger I gave up and started from scratch porting to Sapper.

Finally got it working and same issue! Two days in at this point and realised that the component was only failing to update when the _layout.svelte didn't change, i.e. if I was navigating from /lessons/korean/home-row/1 to /lessons/korean/home-row/2.

Tried various things, I was surprised that reactive function statements didn't solve this by triggering a rerender:

  let lessonCharacters;
  $: fetchLessonData();
  let promptLines;
  $: generatePromptLines();

  function fetchLessonData() {
    lessonCharacters = lessonData[keyboard]["rowLessonGroups"]
      .filter(g => g.metaData.slug === lessonGroup)[0].lessons
      .filter(l => l.metaData.slug === lesson)[0].characters
  }

  function generatePromptLines() {
    promptLines = [...Array(3)].map(x => generateSinglePromptLine());
  }

After ~30hrs trying to figure this one out, I'm so relieved to have stumbled across this issue. Thanks @Jayphen for the convenient solution!

My 2 pence on this issue.. this was a frustrating quirk to deal with, mostly because it's not obvious that this is not a bug. However, I'm not sure if it's a documentation issue or a design issue. I don't know enough about Svelte internals to be able to comment. What I can comment on is the developer-experience—the appeal of Svelte for me is faster iteration times without seemingly any cost in scalability.

One of the things that makes Svelte so productive for me is that I compared with other frameworks I find that it takes 70% less googling (being honest about how I write code here!) in order to figure out how to do something. There's another well-known framework I've used that I find slow-going to work with, because to achieve comparable user-features, it's required for me to make sense of 10 different micro-libraries and then combine them in the right way like a master-(spaghetti)-chef.

Whereas with Svelte, I can code like a scrappy startup founder without worrying too much that I'm creating a giant ball of mud. I put this down to the fact that can use the same core svelte features (stores, reactive statements etc.) to achieve a wide array of different solutions and importantly, many problems that would otherwise require a lot of configs and boilerplate are solved by the library.

For me, it'd be great if there was an internal design-solution to this problem instead of custom configs / workarounds, mostly because I value having to write less lines of code, not just because this frees me up to focus more on shipping value to my customers but because on principle I view every line of code in my repo as technical debt that has to be managed.

from kit.

This just bit me when implementing a /user/[id] route. The admin user can edit details of any user, and a normal user can edit their own details.

When I click from editing a user to the "settings" link so that admin to edit own details, nothing happens, due to this issue. It's only by good luck that I happened to click in that order and noticed this at all.

Information security and privacy is more important than efficiency, and I don't want there to be any possibility that residual data from editing one user can accidentally transfer over to when I am editing another user. The natural expectation is that onMount should be called each time I go to a different route or URL, including different parameters. Each route or virtual page should be a fresh instance of the page without any data left over from the previous route.

I think that this safe and unsurprising behaviour should be the default, or at least we need some option to make it the default. As it is, I'm not too sure how to fix my page. I didn't design it thinking that its parameters might stealthily change underneath it.

Is there some function I can call to remount the page when I detect this circumstance? In addition to just making it work, I am concerned about the possibility of information leaking between the different user pages, so I'd rather "reload" the page than attempt to make it adjust everything when the parameters change, but I don't want to call location.reload() if there's some other option.

I haven't seen any general-purpose and concise working solution above. My current "solution" is an ugly hack:

<script>
        // Detect when sapper is changing the page parameters underneath us, and reload the page.
        // https://github.com/sveltejs/sapper/issues/1278
        let original_path;
$:      if (original_path && $page.path != original_path) reload_page();

        async function reload_page() {
                const hash = window.location.hash;
                console.log("reload_page", original_path, $page.path + hash);
                await goto('/', {replaceState: true});
                goto($page.path + hash, {replaceState: true});
        }

        onMount(async () => {
                original_path = $page.path;
                // ...
        }

        // ...
</script>

from kit.

... it makes it much easier to create appealing page transitions when you're navigating between different pages belonging to the same route

• @Rich-Harris

I totally understand where you're coming from here. I agree 100% that the web has a serious disconnect from other forms of applications when it comes to user experience. The web is, by and large, devoid of continuity and that is unfortunate. There are some legit reasons for this but hopefully we are at, or nearing, the point where this can be rectified.

Your example looks great, especially for such a quick spike, by the way. I think it illustrates what we've been missing in this space. However, it is also restricted to the same page/route. If the expectation is for the web to evolve and become more in line with native experiences, then we need to look at the application as a whole, yea? Presumably, most applications are going to operate with a lot more than a single route.

This means that for that sort of experience, the optimizations of component mount cycles either can't be a deciding factor or they need to be moved up a layer. I think layouts are often over-used but this seems to sit squarely in their domain. A page shouldn't be expected to know how to transition to every other page.

There is an expectation, rightfully or otherwise, that a "Page" is an isolated, end-to-end process. By having circumstances where hooks are not fired upon is alien to the terrain. It seems counter intuitive to need to employ defensive programming to ensure those lifecycle events occur or have occurred.

Now, having said all of that and setting aside the performance improvements, there are usecases for the current behavior. The best example that comes to mind is infinite scrolling, where the current cursor is represented in the URL. However, in that case, it probably makes more sense to avoid load as well. I'd prefer to be able to explicitly opt-out of those life cycle events, even if that is a primitive implementation such as window.location = `/photos?cursor=${cursor}`;.

</twocents>

from kit.

I've encountered the same problem....what's the status of this "bug"?

Please don't ask questions like this as it doesn't add anything except noise to our inboxes. I'd rather spend time contributing to the project than managing my inbox. The issue is open, which indicates the status. Anyone who would like to take a stab at implementing this is free to send a PR

from kit.

Ah that's a very good spot. I'm absolutely certain that the "method of least surprise" is a route change calling create/destroy.

However I realise this is a breaking change, so we have to decide if we leverage the fact that sapper is pre 1.0 and make this breaking change with a sensible default.

Page store is documented in Svelte and in Sapper (preloading)

I don't feel personally that we need new APIs. I think we can possibly add a flag to the <a> tag which will allow a consumer to not have the old page torn down and recreated, if they really want that behaviour.

it's worth noting that if you use a router which uses <svelte:component> which is a large number of Svelte routers, I think, you do get the onMount and onDestroy calls.

from kit.

My use case is quite complicated, so it's difficult to really recreate in a simplistic way, but I'll try. Essentially if you're doing any work in onMount or the initialisation block of the page, this work won't be done for the new information when the route changes, so you can end up with stale data on your page, which is completely unexpected and hard to track down.

(In a simplistic form, you could use the $page store to make this data reactive again, but it's completely unexpected to have to do that - it's not something you'd do automatically, you'd have to know that a route change wouldn't re-run your intialisation code unless you were route-changing from a different page component)

from kit.

A few things to think about:
If we put it on the link then the user has to make sure to add it to every link to the same route. That seems potentially error-prone as I imagine it'd be easy to miss one and would need to be considered every time a new link is added
How would programmatic navigation be handled?
It doesn't allow the user to have a handler both for the first time the component is mounted and for when the page is navigated to.

The gif example in my view is a pretty rare use case and I'm not sure it's the one I'd choose to optimize the API for. If we always destroyed the old component and mounted a new one on navigation (which is more in line with what I'd expect to happen), I think the user could still build the behavior in the gif within a single component by using pushState

In any case, if we want to keep onMount working as it does today, I think something like Routify's beforeUrlChange would be a friendlier way of handling this. Then you only need to set it up once instead of making sure you consider whether every link on the page needs a new attribute. It also makes it easier to have both types of handlers instead of choosing either/or

from kit.

Changing the core behaviour would definitely break stuff that I have built, no question, it would also make those apps impossible to build with sapper. I would personally consider remounting on every route change a step backwards.

What if the current behavior remained available (as I suggested) by passing {remount: false}?

from kit.

The more I think about it, the more I realize that onMount is actually an implementation detail of sapper and should only rarely be used directly at the page level.

I think we should add two different events to each page. Something like 'visit' and 'leave' or 'load' and 'unload'.

IMO this makes it much more obvious what the page author's intent is, and avoids trying to constrain sapper behavior to be less efficient and possibly violate the principle of least surprise when coming from a svelte mindset.

Would these two events be enough to address the needs that folks are experiencing?

from kit.

I think something like this is probably the best solution. The problem stems from onMount being a component level primitive, what we need is a page level primitive that we can use to enforce consistent behaviour relating to routes and pages.

Now the page store does exist but I'm guessing some access to the preload values will be needed. They could possibly be passed to the navigation events as arguments. This may also address another issue about onNavigate.

from kit.

Forcing a remount is one thing

That's what I'm advocating should be the default. That's all.

from kit.

I have to ask: Why is navigating between two dynamic routes of the same kind such a special case that it requires every variable be reactive and subscribed to $page changes? If the performance benefit of not remounting the component is so important, why not mount all components only the first time they are loaded, preserve their state indefinitely, and manage the state of all variables in components using reactivity?

That sounds like a nightmare to me personally and I suspect it may to many of you. I bring it up just to illustrate: what's so bad about remounting the component? We do it between every other transition between views in Sapper and don't think twice about it. And there's many benefits to remounting! You get local state that's easy to reason about, and you opt-in to application state where you need it.

I realize I may be beating a dead horse here, but I just don't get it what is so special about navigating between two dynamic routes of the same type that makes it deserve a completely different programming approach.

from kit.

@clockelliptic It seems you have completely misunderstood this issue.
The preload function is called on every url-change, thus your code makes no sense.

BTW it does not seem that IBM has stopped the development of carbon-components-svelte.

from kit.

@benwoodward A probable reason that the reactive statements did not help here is that the reactive statements themselves do not reference the changing properties.

Svelte doesn't delve into the functions you call to see which properties they use, so Svelte sees the call to fetchLessonData as only depending on the fetchLessonData itself, and so it will only run again if that function itself is somehow reassigned.

You can fix that by doing something like $: fetchLessonData(lessonData[keyboard]) instead. That way Svelte will log the dependency on both lessonData and keyboard and call the function again as appropriate.

from kit.

Hey, I see that this issue was closed but I couldn't figure out what was the resolution?
I see the linked PR but I don't understand how that fixes this issue.

Secondly, I have a little problem with the suggested load() function workaround when using TypeScript.

I have a variable stats that is assigned times during the lifetime of the component (first assignment being on route load).
Something like this:
let stats = getStats();

It uses props data and component data. Doing it this way, I don't need to explicitly specify the type of stats. Now if I have to do it inside load() function then

1. I will have to import the getStats function in both script blocks.
2. I will not have automatic type assignment for stats variable and have to declare its type explicitly

from kit.

I think this problem manifested itself because the pricing indicator uses a store, so it's that store which needs to have it's value re-evaluated.

The content is loaded in preload (as per the demo), but once you click a new link, the assigning of a value to it's display variable (emulating population of a store) doesn't happen. I also tried with a reactive variable but the updated content didn't trigger an update.

I'll see if I can reflect the $page idea in the repro above to see if that improves things.

from kit.

This works differently than I'd expect as well. Though onMount comes from Svelte and maybe we need Sapper-specific APIs for this? There are a lot of open issues around routing and page transitions (e.g. sveltejs/sapper#902, https://github.com/sveltejs/sapper/issues/814, sveltejs/sapper#1083, sveltejs/sapper#518, sveltejs/sapper#295, sveltejs/sapper#674). It seems like it'd be good to consider all of these together and figure out what API changes if any are needed to address most of the use cases

Rich discusses a lot of the issues raised here in sveltejs/sapper#295

from kit.

I mostly agree this isn't the best default as does Rich (in sveltejs/sapper#295 he says "I would argue that destroy/recreate should be the default behaviour")

My main question though is if we want to support both behaviors longer-term, how would we do that? I can think of a few options:

1. Say that onMount and onDestroy are always called for the last part of a route and if you want something that persists across routes then you should put it in the layout common to that route segment which will be persisted instead of recreated
2. Say that the user must handle it via pushState and updating the UI on their own
3. Let onMount and onDestroy operate as they do today, but introduce new APIs like onNavigateTo and onNavigateFrom
   1. That would also allow us to support route guards (though beforeunload might be a better solution for that). E.g. Routify has beforeUrlChange and afterPageLoad. Vue Router has beforeRouteEnter, beforeRouteUpdate, and beforeRouteLeave

Btw, I think the $page store is undocumented because I can't find "$page" in the docs

from kit.

If you go from /a/b/c to /a/b/d would that only call create/destroy on the last segment and not on the layouts for /a and /b?

We have a couple other PRs for breaking changes as well. I think it'd be nice to have a 0.28.0 that merges any outstanding breaking changes. Sapper doesn't feel like it's ready for a 1.0 yet, and I think locking in all the current behaviors and APIs might be a bit premature.

from kit.

After playing with this for a few days, this is definitely a bug. It might be intentional behaviour, but forcing the user to manage the component lifecycle because they visit two routes which use the same component isn't desirable, it's unpredictable, and it's also extremely difficult since you have to do all sorts of manual checking of state.

My belief is that the most sensible behaviour is that route changes should always cause an onMount and onDestroy, unless explicitly overridden.

I don't think anybody would complain if this behaviour was changed for version 0.28. I believe it is the "API changes" we talk about in the docs and README. It also shouldn't break any existing sites with this as the default, since it's just doing a little extra work, and not removing anything.

from kit.

Having said that, I do concede that there's a precedent for exposing options for this sort of thing as attributes on <a> elements (sapper-noscroll, rel="prefetch"). So it could be done there (and maybe rel="prefetch" should become something like sapper-prefetch while we're at it).

But I'd still argue that the current behaviour should be the default and not the option, since a) it's the more efficient way, b) it's working as components ordinarily work (though of course people have asked for an equivalent of React's key for components in the past), and c) I think it's a cool differentiating feature that AFAIK you don't get with other frameworks.

If you go from /a/b/c to /a/b/d would that only call create/destroy on the last segment and not on the layouts for /a and /b?

Definitely think we want to keep this behaviour the same, i.e. layout components persist — I can't imagine a situation where you'd want to recreate the layouts from scratch (and consistency with that is perhaps another argument for keeping the behaviour of the leaf component as it is).

Side-note: While writing this comment I came to regret my giant flashing GIF.

from kit.

Those are both Svelte things though that are used to call code when the component is created and destroyed, and the component is not getting created or destroyed. We should document this, yes, but we shouldn't give Sapper a way to circumvent intended Svelte behavior (nor, I think, could we, without having Sapper use undocumented Svelte APIs). Is there anything that you can't do with a reactive block subscribing to the $page store?

from kit.

Does 'unexpected' mean 'undocumented', or would this still trip people up even if it was covered well in the docs (and any future tutorial/guide we produce)?

we shouldn't give Sapper a way to circumvent intended Svelte behavior (nor, I think, could we, without having Sapper use undocumented Svelte APIs)

We could do it like this, I think:

const OldLeaf = Leaf;
Leaf = null;
await tick();
Leaf = OldLeaf;

If we were to add this feature as a sapper-* attribute on links, what should the attribute be called?

from kit.

If we put it on the link then the user has to make sure to add it to every link to the same route.

That is a concern, yeah. It's a feature of the component rather than links to the component, but we don't have a great way to express that in-band.

How would programmatic navigation be handled?

app.goto(url, { remount: true });

The gif example in my view is a pretty rare use case and I'm not sure it's the one I'd choose to optimize the API for.

This might just be me tilting at windmills, but to me the fact that it's a rare use case is itself the problem. We web folk have trapped ourselves in the 'pages' mindset, where navigation represents a jump from one discrete thing to another. In native apps and in more pie-in-the-sky sources of inspiration, navigation is often more like exploring space or exploring an object, in a way that emphasises fluidity and continuity and more closely resembles how our brains model the real world.

It's true that my example is slightly contrived — I cobbled it together fairly quickly last night — but I don't think it's so hard to come up with better real world ones. Think about a photo album app where you zoom in from the grid view to an individual photo when you tap it, or where the pixel average background colour behind the photo transitions smoothly, or an ecommerce site like this except with similarly appealing transitions when you navigate between alternatives rather than going from index to detail view, or really any app where you're actually navigating through a space (something like this Sketchfab example, except that the 'annotations' are actually URLs).

I think the user could still build the behavior in the gif within a single component by using pushState

We probably want to avoid cornering developers into using the history API directly, since Sapper is managing the history — it's a likely source of bugs. At the very least I think we'd want to wrap that API (i.e. handle via app.goto).

In any case, if we want to keep onMount working as it does today, I think something like Routify's beforeUrlChange would be a friendlier way of handling this.

Actually yeah, lifecycle functions might be the correct way to handle this. I don't think they need to be implemented as stores as they have been in Routify, they could just be normal custom lifecycle functions AFAICT.

from kit.

Actually yeah, lifecycle functions might be the correct way to handle this.

Would this mean that I would have to consider use of onMount in page components, instead preferring (say) beforeUrlChange, since only one is guaranteed to run when a page loads?

from kit.

think about a photo album app, ... an ecommerce site like this, ... something like this Sketchfab example

Good examples! The way I had been thinking this would work if we did remount each component upon navigation is that you would have a layout component (potentially with no UI and just code if that's all you need) that could have it's own onMount. That would end up being run the first time you navigate to the photo album, for example, but not when opening an individual photo. I think that would end up covering both the traditional page-based use case Antony raised as well as these more interactive use cases

Would this mean that I would have to consider use of onMount in page components, instead preferring (say) beforeUrlChange, since only one is guaranteed to run when a page loads?

My thinking was that you would use the one appropriate for the usecase. For the usecase you have where you want it to run everytime a page loads then you'd put the code in beforeUrlChange. If you want something to run just once when first loading the component, but not for individual page navigations, then you'd put it in onMount

from kit.

Yeah, I'm envisaging something like this, perhaps:

// @sapper/app
import { onMount, tick } from 'svelte';

// ...`stores`, etc

export function onNavigate(fn) {
  const { page } = stores(); // this works because of when `onNavigate` is called
  
  onMount(() => { // ditto
    const unsubscribe = page.subscribe(async $page => {
      await tick(); // so that it matches `onMount` semantics following subsequent navigations
      fn(); // or fn($page)?
    });

    return unsubscribe;
  });
}

from kit.

Could we also add a function like onLeave or onBeforeNavigate as well? Would it just call fn() without calling tick() first? I haven't used stores much yet...

from kit.

Yeah, I'm envisaging something like this, perhaps:

// @sapper/app
import { onMount, tick } from 'svelte';

// ...`stores`, etc

export function onNavigate(fn) {
  const { page } = stores(); // this works because of when `onNavigate` is called
  
  onMount(() => { // ditto
    const unsubscribe = page.subscribe(async $page => {
      await tick(); // so that it matches `onMount` semantics following subsequent navigations
      fn(); // or fn($page)?
    });

    return unsubscribe;
  });
}

That would never unsubscribe the onMount listeners, would it?
Also, I've created a PR about 6 months ago (sveltejs/sapper#1047), with a onNavigate function to prevent navigation. Maybe this could also resolve this issue.

Or maybe just export a constant in the module script block, like the preload function, to tell sapper that this component should remount.

<script context="module">
  export const REMOUNT = true;
</script>

Then the normal onMount/onDestroy functions would be called on remount with this:

We could do it like this, I think:

const OldLeaf = Leaf;
Leaf = null;
await tick();
Leaf = OldLeaf;

from kit.

This might just be me tilting at windmills, but to me the fact that it's a rare use case is itself the problem. We web folk have trapped ourselves in the 'pages' mindset, where navigation represents a jump from one discrete thing to another. In native apps and in more pie-in-the-sky sources of inspiration, navigation is often more like exploring space or exploring an object, in a way that emphasises fluidity and continuity and more closely resembles how our brains model the real world.

You might cobbled your example quickly, but in my use case it totally makes sense the way it works right now. I'm developing a TV ui, which has to include and initialize the video tag on reaching the /tv/channel path, and just interact with the tag when you switch channels to /tv/channel2. Subscribing to the page store to get the updated channel number is enough for this case, destroying and recreating everything would be excessive, and in my case that runs on very low performance chips, a dealbreaker.

from kit.

The problem is onMount is a svelte thing. And the component isn't remounting. So it definitely isn't a bug, 100%.

It might be confusing, it might be undesirable to some extent, but it isn't a bug.

Changing the core behaviour would definitely break stuff that I have built, no question, it would also make those apps impossible to build with sapper. I would personally consider remounting on every route change a step backwards.

I have little opinion on whether or not a way to force a remount would be good for sapper. I see no harm in providing that option if people need it.

from kit.

I don't think the less performant solution, that circumvents Svelte's default behaviour, should be the default solution.

from kit.

Sapper docs say that goto "Programmatically navigates to the given href." That's not what goto does if a user gets two different results depending on where she is coming from and going to.

from kit.

As far as I can tell there isn't much (any?) difference between using an anchor or goto in this scenario. This is just about how sapper handles routes changes in general, which in my opinion is fine. We need to communicate it better, maybe we need solutions to the problems that people are reaching for onMount for, but the behaviour itself is very useful.

I don't think goto needs to emulate a hard refresh, nor do other route transitions. I'm not against configuration to modify this behaviour.

from kit.

Again quoting the Sapper docs here: "In other words, the behaviour is as though the user clicked on a link with this href."

from kit.

I might be misunderstanding what it means to have different results depending on how a user got to a particular page, but isn't this the nature of working in a reactive environment?

There are an almost unlimited number of updates that could get you to a given state, but the compiler/runtime/compute paradigm manage all of these on your behalf.

It seems like there are some holdover expectations about the set of side effects that will precede entering a particular state.

Perhaps there are a set of invariants and/or events we can identify that are a better fit for this reactive world? Is it as simple as having a better set of beforeNavigateHere / beforeNavigateAway events on page components?

Perhaps this would better align the expectations folks have around when onMount is fired?

from kit.

Why don't we quote the whole thing:

Programmatically navigates to the given href. If the destination is a Sapper route, Sapper will handle the navigation, otherwise the page will be reloaded with the new href. In other words, the behaviour is as though the user clicked on a link with this href.

So that is clearly talking about a number of things and does not anywhere suggest that the navigation replicates default browser behaviour, except when the url is external to the site. Otherwise it is "as if the user has clicked on a href" which has very specific behaviour when that link is an internal link. Client routers in no way replicate default browser behaviour.

Regardless, we are talking about what is desirable, not how good the documentation is. We have already clarified that the docs could be improved.

from kit.

from kit.

from kit.

Sapper wouldn't ignore onMount, we would simply need to document that when onMount triggers might not always meet your expectations and point people to dedicated APIs for page transitions.

Sapper has the responsibility to honor all of the guarantees that Svelte put forth, including running onMount when a component is loaded. Even if under the hood Sapper isn't really loading the component because it's smart enough to know that its really dealing with two dynamic routes of the same type, that doesn't really matter.

So Sapper has the responsibility to honour all of the guarantees that Svelte puts forth but to modify them confusingly, in a way that doesn't actually reflect what Sapper is actually doing? Forcing a remount is one thing but monkey-patching onMount so it behaves differently in Sapper is never going to happen.

The whole issue has come about because Sapper does honour all of the guarantees that Svelte provides. The above proposal suggests a dedicated API so you don't need to concern yourself with the inner-workings of Svelte.

As an aside, onMount should really only be used when you need a guarantee that the component has mounted. Client only behaviours, data fetching, etc should be handled elsewhere, those docs need updating.

from kit.

Apart from being a breaking change, I've explained why that is a bad idea. Rather than hamstringing Sapper, we can look at dedicated APIs for this specific problem.

from kit.

I think it's less about what's special in the scenario you've outlined, and more about the fact that sapper delegates all component lifecycle management to svelte, which is free to optimize component mount/unmount/update, etc.

In the ideal case, there would be no wasted effort of any kind: no remounting, no unmounting and then mounting.

I think sapper is trying hard to give the appearance of a single cohesive application while also fitting the page/webapp metaphor.

The fact that we sometimes have a preference for a particular sequence of lifecycle events suggests that an abstraction leak is happen somewhere. (Probably at least partially in sapper.)

More concretely, I think if anyone cares about how many times mount is called (beyond the barest minimum), they are likely depending of some side-effect of their onMount code.

I think we should enumerate the needs that are driving us to depend on side-effects and address them in a more direct and less implementation-detail-sensitive manner.

from kit.

Sounds great to me! I would advocate for names that make it obvious when the event will fire. So like onBeforeExit or onWillExit or like that PR suggests, onBeforeNavigate or even onWillNavigate

from kit.

Sounds great to me! I would advocate for names that make it obvious when the event will fire. So like onBeforeExit or onWillExit or like that PR suggests, onBeforeNavigate or even onWillNavigate.

from kit.

Another idea for solving from @Jayphen on Discord:

Personally I use afterUpdate combined with a store I created that tracks the current and previous pathname
Then afterUpdate I check if the pathname changed. If it did, it was a route change. (I use this for page tracking)

from kit.

To elaborate on the above, here is a gist example. I use this for route-level page tracking

https://gist.github.com/Jayphen/dc2db7591cd0735cefa586b2f5facacf

(I take no responsibility for the quality of this solution. It hasn't passed code review yet)

from kit.

I've noticed that my syntax highlighting in code blocks is removed after the first page visit. Prism.highlightAll() only runs on the first page load. Any tips towards a work-around?

from kit.

from kit.

I've been bugged out about this for a while and found a quick workaround that works for me.
Just add the following to your code:

import { stores } from "@sapper/app";
const { page } = stores();
$: if (page) {}

This triggers an update when the route changes.

from kit.

@Rukkaitto That doesn't seem to work for me. I want to refresh my auth token on any route change. Here's my code so far:

<script>
  import { stores } from "@sapper/app";
  import { onMount } from "svelte";
  import Nav from "components/Nav.svelte";
  import Footer from "components/footer.svelte";
  import Sidebar from "components/sidebar.svelte";
  import { jsonHeaders } from "./server/_helpers";
  // export let segment;
  let sidebarShow = false;

  const { session, page } = stores();

  const refreshToken = async () => {
    // only refresh when signed in (token is not empty)
    if (!!$session.token) {
      console.log("Old token: " + $session.token);

      const res = await fetch("/server/auth/refresh", {
        method: "POST",
        headers: jsonHeaders({}),
      });

      const result = await res.json();
      const { token } = result;
      $session.token = token;

      console.log("Refreshed token: " + $session.token);
    }
  };

  $: if (page) {
    refreshToken();
  }
</script>

I place it at _layout.svelte .

Anyone got a solution? This is working only when I refresh the page.

I also agree with the OP of this issue. Maybe create an equivalent of onMount that's triggered on goto's and redirect's ?

from kit.

I just bumped into the same problem. I read that changing the behavior is a breaking change. This issue is not on the 1.0 milestone though. Shouldn't it be on the 1.0 milestone?

I bumped into the problem by loading data based on a page slug in the onMount function. I used goto to route to a different page slug e.g. another project id and expected the data to update, which did not happen due to this issue.

from kit.

I've encountered the same problem....what's the status of this "bug"?

Just to give an idea of another scenario, this is my use case:
I've a breadcrumb for the category tree which does not get updated, but it shows on the bottom-most category the items (or subcategories) with that category when navigating through pages. Both category tree and items are taken from the same load function. Sometimes the items export will be an empty array, sometimes it will be an array populated with objects.

from kit.

The maintainers have been having some discussion about whether a new component should be created when you navigate to a new URL corresponding to the same route and are looking for input from the community: #5007

from kit.