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

Support for Shallow Routing (from Next.js) #2673

Closed
ghost opened this issue Oct 23, 2021 · 25 comments
Closed

Support for Shallow Routing (from Next.js) #2673

ghost opened this issue Oct 23, 2021 · 25 comments
Labels
feature / enhancement New feature or request
Milestone

Comments

@ghost
Copy link

ghost commented Oct 23, 2021

Describe the problem

See: https://nextjs.org/docs/routing/shallow-routing

Shallow routing allows you to change the URL without running data fetching methods again, (...) [it] only works for same page URL changes.

Describe the proposed solution

import { goto } from '$app/navigation';
await goto(href, { shallow: true });

plus

<a sveltekit:shallow {href}></a>

These would update the page store, but load would not run. (Only if href does not link to a new page)

Alternatives considered

Using the History API directly:

history.replaceState(history.state, document.title, href)

But doing this does not update the page store (and it's ugly).

Importance

would make my life easier

Additional Information

I have this OrgTree component which lazy loads it's items as you expand the groups:

20211023151814

When you click an item the URI is updated using shallow routing, leaving all the loaded tree data intact.

Using Svelte Kit, load fetches the initial tree data, but, after navigating, the initial data is loaded again, reseting the whole tree.

Related issues: #1930, #969

@rmunn
Copy link
Contributor

rmunn commented Oct 25, 2021

Also related: #2301

@tv42
Copy link

tv42 commented Dec 1, 2021

Wouldn't it be a conceptually simpler model to put your tree data in a store, and have each load just populate more and more of the tree?

@ghost
Copy link
Author

ghost commented Dec 1, 2021

I guess the problem is that, after load runs for the first time (fetching the initial tree structure), if it runs again (because the user selected a tree item which was fetched from expanding a tree group), it's guaranteed that it will fetch unneeded data since the tree item already exists.

@tv42
Copy link

tv42 commented Dec 2, 2021

Your load could look at the store and only fetch things if it's missing.

To avoid load becoming reactive to the store and running every time the store changes, I believe you need to do get(myStore) to get a one-time snapshot of the value.

@Rich-Harris
Copy link
Member

shallow: true feels slightly cryptic to me — I think I'd prefer navigate: false. Though that doesn't really work as an attribute on the <a>.

As mentioned in #969 (comment), one use case for this could be updating query parameters without causing navigation (which has unwanted side-effects).

@Rich-Harris Rich-Harris added this to the 1.0 milestone Apr 7, 2022
@bitdom8

This comment was marked as off-topic.

@madeleineostoja
Copy link

doesn't goto('/path', { replaceState: true }) already achieve most of this?

@Rich-Harris
Copy link
Member

Most, but it causes load to re-run, which is what the OP is trying to avoid

@Rich-Harris Rich-Harris modified the milestones: 1.0, post-1.0 Aug 27, 2022
@dummdidumm
Copy link
Member

Some thoughts:

This could be a new option on goto named skipLoad, which, as the name says, would skip calling the load function. You could get variances of shallow routing through the goto option then: goto(url, { replaceState: true, noScroll: true, keepFocus: true, skipLoad: true }) would be the most "shallowest" of routings. An accompagning data-sveltekit-skipload would do the same for <a> links, though there are no attributes for the other goto options (as also requested in #7895).
One question to answer (or maybe in general? this applies to other options today) is what should happen on a popstate event. If you navigated to the URL with skipLoad, should loading be skipped on going back, too?

The alternative would be some kind of shallow: true / data-sveltekit-shallow option which basically enables all these flags at once.

I'm not sure which solution would be more viable in practice. The first option has more flexibility, but do people need that flexibility?

dummdidumm added a commit that referenced this issue Jan 25, 2023
@Rich-Harris
Copy link
Member

I'm not sure skipLoad is the right thing, conceptually. It's not about skipping load — the use cases here are really about appending to the history stack without triggering navigation.

Three use cases:

  • Showing a modal that the user can dismiss by hitting the 'back' button. In this case the URL will stay the same, and we don't want to load any data or reset the scroll etc, we just want to create a new history entry with some user-defined state (e.g. { modal: true }) associated with it
  • Instagram-style navigation, where clicking on a photo in a list on a page like /[username] shows an expanded view inline and changes the URL to /p/[photoid]. If you hit 'back', you go back to the previous view. Scroll is preserved throughout. if you reload the page, you go to the /p/[photoid] route. (Unfortunately if you then go back, the app breaks, because they're clever but not that clever. SvelteKit will handle this case correctly.)
  • The OP's example, where you're clicking around some piece of UI and updating some data, but you don't want to call load. In the example given, you probably don't want to reset scroll

I'm not pretending to have figured out all the details, but here are some high-level thoughts:

In all these cases, we're using history.pushState rather than replaceState, we're not resetting scroll or focus, and we're not calling load or rendering new routes. In fact I'm not sure goto is the right function for this at all — if we need to add one option, ignore another, and disable the defaults on two more, then it's probably a different construct.

For the sake of argument, let's call that new function pushState. (Probably less confusing to come up with a new name but I can't think of one right now.) Calling it would update $page.state. (Perhaps it also updates $page.url, though given that we're not navigating, $page.params and $page.route.id probably shouldn't change, and those things are all connected so it might be kinda weird.)

Usage:

<script>
  import { pushState } from '$app/navigation';
  import { page } from '$app/stores';
</script>

<button on:click={() => pushState('.', { modal: true })>
  show modal
</button>

{#if $page.state?.modal}
  <div class="modal">...</div>
{/if}
{#each photos as photo}
  <a
    href="/p/{photo.id}
    on:click|preventDefault={() => {
      pushState(`/p/${photo.id}`, {
        photo
      });
    }}
  >
    <img alt={photo.alt} src={photo.thumbnail}>
  </a>
{/each}

{#if $page.state?.photo}
  <div class="expanded">
    <img alt={$page.state.photo.alt} src={$page.state.photo.src}>
  </div>
{/if}

We probably also want to call beforeNavigate and afterNavigate hooks.

@emmbm
Copy link

emmbm commented Jan 26, 2023

I can think of cases where one could want to update the client url's search params but without running any beforeNavigate or afterNavigate hooks. It might be relevant to offer finer granularity of what side effects the state update has, if that's possible.

@Rich-Harris
Copy link
Member

If we were to call beforeNavigate and afterNavigate callbacks, the navigation object passed to them would have a type property you could use. Though on the basis that these aren't navigations, we could certainly argue against calling them

@dummdidumm
Copy link
Member

dummdidumm commented Jan 27, 2023

I don't think we can get away with just one opionated function for this:

Conflicting opinions/needs regarding to history:

Conflicting opinions/needs regarding $page updates:

Conflicting opinions/needs regarding load:

Conflicting opinions regarding before/afterNavigate:

  • OP wouldn't want to rerun these
  • The Instagram would maybe benefit from that: You're logged out, click on that image, app aborts and shows a login modal instead first

Furthermore, a new function that is almost goto with a set of flags turned off an on feels strange. With skipLoad:

(I would be ok with a helper function that wraps goto with a opinionated set of flags turned on and off, but you could just as well create that yourself; and as shown, there might not be one correct answer.)

Looking at the example code Rich gave, I'm not even sure why pushState would be needed as a concept, since the same can be achieved with history.pushState + a local variable (or a store in context, if upper layouts need to get a hold of it). Furthermore, as mentioned above already, in the Instagram "open details in modal" case you do want to run load functions, you probably want to reuse most if not all of the original details/+page.svelte - which sounds more like NextJS' intercepting routes proposal. So maybe the disagreement comes from understanding the feature request as two different sets of issues.

@flut1
Copy link

flut1 commented Jan 27, 2023

I read through some of the use cases in this issue and #969, and I agree with @Rich-Harris that the common use case seems to be changing the URL without triggering navigation. Here's what I observe in most1 use cases:

  • We want to update the URL to reflect some state change on the page (e.g. a viewing preference was changed), in order to preserve this state when the URL is shared.
  • This state change could be triggered by anything, for example: user clicks a photo to view details, types in a text input, or clicks a button
  • The URL change can be anything that still resolves to the same "page", such as updating a query parameter or adding a sub-route
  • Sometimes we want a new history entry to be created (pushState), sometimes we don't (replaceState).

It seems to me that what we're really trying to achieve is that changes to certain parts of the URL shouldn't trigger a navigation, regardless of how that URL change was triggered in the first place (pushState(), replaceState(), clicking a link, browser forward/back, goto()).

I can almost achieve this behavior using a check in beforeNavigate() and calling cancel() if navigation shouldn't occur. However, this also reverts the change to the URL. So how about something like adding an option to cancel() to opt out of that behavior?

beforeNavigate(({ to, from, cancel }) => {
    if (!someCheckIfNavigationShouldOccur(from, to)) {
        cancel({ cancelHistoryChange: false });
        
        // some code to update the view can be added here
        // or parts of the page could subscribe to changes in the $page store
    }
});

I'm not sure if adding an option to cancel() is the right way to go or if this should be handled in beforeNavigate() at all. I can imagine that writing a check inside your beforeNavigate callback to test if only a certain part of the URL has changed can get a little messy. My main suggestion though is that it would be nice to have a way to opt out of navigation on certain URL changes, rather than providing option(s) to goto or using a different function to change the url.

Footnotes

  1. one exception is the instagram example @Rich-Harris pointed out: you want to navigate to a new page (and call load) but display that page's content within the current page. I agree with @dummdidumm that this is really a different feature request and should probably be a different issue.

@Rich-Harris
Copy link
Member

I don't think we can say that an intercepted navigation to /p/[photoid] wants to run the load function for that route — the intercepted view might look quite different than the full page view, and rely on different data. I'd envisaged using an API route to fetch any missing data (since it's likely that you already have some, e.g. the photo object, and don't need to load it again). It really depends how declarative vs imperative we want this to be, where 'declarative' means something like Next's boob syntax — (..) — and 'imperative' means putting the logic directly in components and event handlers as in the above illustration.

If we were to treat intercepted routes as a distinct feature though, then all the cases we're discussing where you'd want to skip load functions are goto('.', '...'), which is borderline oxymoronic — you can't 'go' to the same place you're currently at. If goto(anywhere_else, { skipLoad: true }) is an error, that's a strong indication that it should actually be a different function.

I actually think updating query parameters is the special case, rather than intercepted routes, because it's the one case where you want $page.url to update. Perhaps we need a separate function for that:

import { updateQuery } from '$app/navigation';

function search(q) {
  updateQuery({ q });

  // if it takes the same argument as URLSearchParams these would also work
  updateQuery(`q=${q}`);

  const params = new URLSearchParams();
  params.set('q', q);
  updateQuery(params);
}

@Rich-Harris
Copy link
Member

(One thing I will say for boob syntax, even though I lean in the other direction, is that it would make code-splitting easier — too finicky to avoid eagerly loading the full <Photo> component otherwise)

@iansinnott
Copy link

I actually think updating query parameters is the special case

It's definitely a common one, but it's not the only case so I'd suggest not limiting the developer to only query params. I've had occasion in the past to use Next.js shallow routing for something like /resource/new -> /resource/[id].

@dummdidumm
Copy link
Member

@iansinnott how did you handle the loading part of new -> [id]? If there's something like skipLoad, and [id] has a load function, that one still would need to run so your export let data stuff is not empty unexpectedly.

I also think special-casing this to query params is too limited. It will expand the API surface with another method that will not suffice, so we need to expand the API some more afterwards. Something like skipLoad: true would handle the query params case, the "don't do anything at all case, I just want to update the history state" (for which you're better off with local component variables anyway in my opinion), and the "go to a different route" case (although this would only work if there's no load function to run on the new route).

@Rich-Harris
Copy link
Member

and the "go to a different route" case (although this would only work if there's no load function to run on the new route)

In the Instagram navigation case you're not skipping load, you're skipping navigation. The 'background' of the page remains the same, which means $page.route and $page.params (and by extension $page.url) must also stay the same. So skipLoad is definitely the wrong API in that case.

The modal case is identical to the Instagram case, except that the URL doesn't change. So I'd strongly argue that skipLoad is the wrong API there too.

In fact the only time $page.url should actually be changing is the updating query params case, which is why I think it's the odd one out here and deserves separate API treatment.

@iansinnott can you describe exactly what should happen when you go from /resource/new to /resource/[id], and the situation in which it arises — is this something that happens when you hit 'save' on /resource/new or something like that? Should $page.route and $page.params update? (My hunch is that they should never update unless you're actually navigating to a new route, i.e. rendering new components.)

@dummdidumm
Copy link
Member

In the Instagram navigation case you're not skipping load, you're skipping navigation. The 'background' of the page remains the same, which means $page.route and $page.params (and by extension $page.url) must also stay the same. So skipLoad is definitely the wrong API in that case.

The modal case is identical to the Instagram case, except that the URL doesn't change. So I'd strongly argue that skipLoad is the wrong API there too.

You don't need any new API for this at all then, you can solve it through calling history.pushState(..) and updating a local variable yourself

<script>
  let open = false;
</script>

<img  src=".." on:click={() => { open = true; history.pushState('/photos/id'); } />

{#if open}
  <div class="modal">...</div>
{/if}

In fact the only time $page.url should actually be changing is the updating query params case, which is why I think it's the odd one out here and deserves separate API treatment.

... which would be solved through skipLoad, which would also solve other use cases like "I actually want the page store to update but not load again". I linked some issue above, where people would prefer to use replaceState instead of pushState, which means at least this flag would also be needed in the new API, and some might want to have the scroll reset or not, etc, at which point we're reintroducing all the options goto already has. If I would write the docs and it would say "If you want to update $page.ulr but not rerun load functions use this other API with otherwise identical options" that would feel weird to me.

@Rich-Harris
Copy link
Member

You don't need any new API for this at all then, you can solve it through calling history.pushState(..) and updating a local variable yourself

Not exactly — you also need to create a popstate listener that will set open = false at the appropriate time, and the new history entry won't be able to use SvelteKit's scroll restoration etc.

I can't think of a situation where you'd want to reset scroll or focus — those are things that happen when you navigate. So it's really just replaceState, and I sort of wonder if we should just do this instead of making it an option:

import { goto, pushState, replaceState, updateQuery } from '$app/navigation';

pushState({ photo }, `/p/${photo.id}`);
pushState({ modal: true });
replaceState({ selected });
updateQuery({ q });

Having functions with clear nomenclature and purpose feels much nicer than making the already-convoluted goto API even more so.

@dummdidumm
Copy link
Member

What's the difference between pushState({ photo }, '/p/photoid') and goto('/p/photoid', { state: { photo } }) in your example?

@dummdidumm
Copy link
Member

dummdidumm commented Apr 19, 2023

Coming back to the instagram-photo case - following idea:

  • add a property to load functions that tells it whether or not it's used in a "embedded" setting, and if yes in which one
  • add a data-sveltekit-intercept="<key>" attribute which you can place on links to tell that this route is now intercepted (will set the corresponding load property)
  • add a <slot name="<key>"> tag at the position in +page.svelte where you want to render the embedded site
  • your own logic to display the slot conditionally
<!-- /photo/+page.svelte -->
<a href="/photo/1" data-sveltekit-intercept="photo">
  <img src=".." />
</a>

{#if !$page.url.pathname.endsWith('/photo')}
  <Modal>
    <slot name="photo" />
  <Modal>
{/if}
<!-- or, if you move the modal logic into the page, just -->
<slot name="photo" />
// photos/[photo]/+page.js
export async function load({ embedded, fetch, params }) {
  const details = await fetch(`photostuff/${params.photo}`);
  return {
   embedded,
   details: await details.json()
  }
}
<!-- photos/[photo]/+page.svelte -->
<script>
  export let data;
</script>

{#if !data.embedded}
  <h1>Details for photo {data.details.name}</h1>
{/if}

...

Advantages:

  • you can reuse most of your logic of the page, because I think they will be similar most of the time
  • no additional folder syntax/API to learn
  • clear when something is intercepted through the data- attribute (though we probably also need some kind of programmatic solution)
  • code splittable

TODO/drawbacks:

  • would using named slots block us from using slots for different use cases in the future (like Treat layout pages like components when using named slots #627)? Using a default <slot> is not an option as it makes this impossible to use in +layout.svelte as that is already reserved for the page content, and you can't slot different things at once
  • this bets on this feature not being used for one page in dozens of places, because all that if-else could get out of hand at some point. I think it's the right bet, the common case is reusing most if not all of the UI/data loading logic in at most two places
  • this also bets on this feature always being used for something you can access via a real link in a non-embedded setting. I think this is also the right bet and plays into our "should always work without JS"-story
  • you always have to explicitly tell SvelteKit that it enters embedded mode. NextJS' intercepted routes have that implicitly just by presence of adjacent files in specific folders. But I argue that it's more visible this way what happens, and it also gives you more control in the rare case you may want to for example only load some photos in the modal but not all

This all is closely related to #9625, maybe more than the simple shallow routing case?

@Rich-Harris
Copy link
Member

What's the difference between pushState({ photo }, '/p/photoid') and goto('/p/photoid', { state: { photo } }) in your example?

goto would navigate (i.e. load the components/modules for /p/[photoid], run load functions, update the DOM, update $page.route etc, reset focus) while pushState would just update the URL and update $page.state.

I think the declarative idea (data-sveltekit-intercept) is interesting, but I'm not sure it would be possible to implement given how slots work in Svelte today. But I also think we probably want more flexibility — we don't always want to be forced to load data (we might have what we need in memory already) or embed a +page.svelte. A programmatic API like pushState serves the three use cases outlined above equally well, and also lets us do trickier stuff where there isn't a 1:1 relationship between a link and some piece of UI.

For everyone following this thread: I've implemented pushState and replaceState in #9847. It works well, and it's a fairly modest addition implementation-wise (though it does introduce a client-side dependency on devalue.stringify, so that we can serialize $page.state to sessionStorage). I think it's a pretty solid approach.

@dummdidumm
Copy link
Member

dummdidumm commented May 11, 2023

One things that's annoying and surprising/hard to keep track of with this approach of pushState/replaceState: You can't use invalidate(All) (and consequently use:enhance) anymore like you would expect within the shallow route, because the moment you do that the shallow route is closed because to SvelteKit the state of things is that you're still on the previous URL.
I think it would make sense to handle this case inside invalidate(All) somehow. The problem is: How to know if invalidate(All) is called from within the shallow route or from within the previous url?
This goes into "parallel routes" territory which makes sense because in some sense a shallow route is a parallel route to the current one - doesn't need to be a modal that overlays the current page, could also be opened to the side and you could navigate both things at once. So in order to solve this there would have to be some notion of which data/invalidate(All)/etc belongs to which (shallow) route.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature / enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants