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

Shallow routing with pushState and replaceState #9847

Closed
wants to merge 22 commits into from
Closed

Conversation

Rich-Harris
Copy link
Member

@Rich-Harris Rich-Harris commented May 4, 2023

This implements the pushState/replaceState idea from #2673.

It adds a new $page.state object which can be set by calling pushState(state, url?) or replaceState(state, url?) (if the url is omitted, it defaults to the current URL). This object is linked to the created/replaced history entry, meaning that if you hit the back button, the state reverts to what it was before. On the server, $page.state is always {}.

This is not the same as goto. goto causes a navigation, meaning we import the components for the next page, load its data, update $page.url, $page.params and $page.route, and reset scroll/focus — none of which we want in the pushState/replaceState case.

The key insight here is that we need to track the history stack and the navigation stack separately. Previously they were conflated, which was mostly harmless (though we need special kludgy handling for popstate events that only changed the URL hash), but that doesn't suffice if some history entries are associated with navigations and some aren't. In this PR, a typical navigation will increment the history index and the navigation index together, but a pushState will only increment the history index.

An alternative API would be goto(url, { navigate: false }) (or skipLoad: true in #2637, though as mentioned load is only one part of navigation), but I think it's conceptually quite different, and this PR shows that the underlying implementation is too. For that reason I prefer a purpose-built API.

Another alternative proposal was to add a data-sveltekit-intercept attribute which caused the link in question to open in a named slot. I'm not sure this would be possible to implement given how slots currently work in Svelte, but in any case I think the programmatic approach will ultimately prove more flexible — if you wanted to implement something like this Y2K article, where each link takes you successively deeper to an unknown depth, it would be relatively easy to implement programmatically but (I suspect) quite a bit harder declaratively:

image

I'm trying this out in an app I'm building, and it works rather nicely. Here's some code for showing a modal:

<label>
  <input
    type="file"
    name="file"
    accept=".jpg,.jpeg,.png"
    required
    on:change={(e) => {
      file = e.currentTarget.files?.[0];

      if (file) {
        pushState({ show_uploader: true });
      }
    }}
  />

  <button>upload</button>
</label>

{#if $page.state.show_uploader}
  <Modal on:close={() => history.back()}>
    <!-- ... -->
  </Modal>
{/if} 

image

I tried implementing this without pushState and it was basically impossible, because as soon as you start adding your own history entries, SvelteKit doesn't understand when it needs to navigate.

preloadData

I'm also using pushState to implement Instagram-style navigation, where clicking on an item shows the detailed view inside a modal instead of navigating to the page. In this case, I want to load the detailed data (including comments etc), but I don't want to reimplement the data loading code (or move stuff into an API route, etc).

At the same time, I want the existing preload logic to work — if I hover over a link and SvelteKit starts eagerly fetching the data (intelligently, i.e. not fetching still-valid layout data) in anticipation of a navigation, I want to be able to reuse the stuff it preloaded.

It turns out we can solve both problems by adding a return value to preloadData. Here's some code for intercepting a click on a photo and showing the detailed view in a modal:

<a
  href="/{item.name}/{item.id}"
  on:click={async (e) => {
    e.preventDefault();

    const { href } = e.currentTarget;
    const result = await preloadData(href);

    if (result.type === 'loaded' && result.status === 200) {
      pushState({ selected: result.data }, href);
    } else {
      // something bad happened! try navigating
      goto(href);
    }
  }}
>
  <Image photo={item} />
</a>

On the same page:

{#if $page.state.selected}
  <Modal on:close={() => history.back()}>
    <PhotoPage data={$page.state.selected} />
  </Modal>
{/if}

Here, <PhotoPage> is an imported +page.svelte page. I'm being lazy by using a static import, but it could equally be imported dynamically so that we can use code-splitting if we so chose.

Because result.data is the App.PageData object that would be passed to +page.svelte normally, data={$page.state.selected} means the page receives identical data to what it normally would. That said, if we wanted to expose an embedded prop or similar in order to change something on the page, we easily could — it's all within the app developer's control.

The page in question does need to be aware of its surroundings — $page.route will differ between the modal and full-page cases, and <form use:enhance> will effectively reload the page because invalidateAll() will load data for location.href rather than $page.url (though maybe we should change that?) — you need to provide a custom use:enhance callback to get the best results. I think this is an acceptable trade-off.

image

updateQuery

Something we talked about in #2673 that isn't implemented here is a separate updateQuery function. The desire is to update $page.url.searchParams without causing a navigation. I haven't thought that all the way through so have omitted it for now.

goto(url, { state })

The existing goto function takes a state option, which is somewhat vestigial. It is added to history.state, but messily — SvelteKit's internal history index is rudely shoved onto the object. For the reasons articulated above, there's nothing useful you can actually do with this state, as far as I'm aware.

So there's a couple of options:

  1. Treat goto(url, { state }) as deprecated and discourage its use. Don't use it to set $page.state
  2. Do use it to set $page.state, in which case we can't deprecate it.

The question is whether there are cases where you want to be able to set $page.state at the same time as causing a programmatic navigation. I'm not sure. If we did deprecate it (which sounds appealing) then you could always do this:

await goto(url);
replaceState(state);

handling reloads

Right now there's some bugginess around reloads. If you open a photo in a modal and refresh the page, you go to the full page (as intended), and hitting the back button will take you to the list page you came from, but only because there's special handling for that case. If you open a photo modal and click from there to a new page, then reload that page, clicking back twice won't work as expected. Still looking into how to fix this.


TODO

  • bikeshed the names?
  • Add App.PageState
  • add descriptions in ambient.d.ts
  • add updateQuery?
  • figure out if goto(url, { state }) should be deprecated or not
  • probably nuke navigation index history on page reload? unless there's a better way
  • intercept history.pushState and history.replaceState to discourage their use
  • invalidateAll() should probably use $page.url.href rather than location.href, since those two things can now be different
  • docs
  • tests

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:.

@changeset-bot
Copy link

changeset-bot bot commented May 4, 2023

⚠️ No Changeset found

Latest commit: a959eb7

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@davecalleja-casumo
Copy link

👀

@samal-rasmussen
Copy link

samal-rasmussen commented Aug 9, 2023

It's really hard to make a webapp that feels both app-like and web-like without this.

I have a lot of views that I want to just open in a temporary overlay when you navigate to it through the ui, because this feels very app-like. But I want the url to reflect the thing itself, so that if you share the url, then thing itself opens directly to its own page, because this feels very web-like.

@dave-calleja
Copy link

Please excuse ping @Rich-Harris 😄 I was wondering if there is an ETA for this feature to see if its worth waiting or making a custom implementation ourselves. Thanks!

@joehenry087
Copy link

After reading through this thread and it's predecessor 2673 and seeing the careful forethought that has gone into this feature, I think it's important to note the importance of the query parameters feature, possibly even over shallow path changing - especially as relates to a data organization and user experience perspective.

Similar to database design, it's usually better to have one index point to one item, and also a URL representing one singular and primary page of a website. Once a URL can point to two totally different pages of a website (or a user can be on two different pages and see the same URL), user experience can suffer - if you've ever shared the wrong link intending to send the main content page, but instead sent the modal content page, you'll know what I'm getting at.

I'd say for most if not all of the use cases described in these threads it's the same, and argue instagram has a flawed design. Is there any case where it wouldn't be better for instagram to use a query param for the modal content? Then when someone send the link, the recipient will see exactly what the sender saw - the main page behind with the modal content intended to be shared above - good UX 👍 There's also many shopping sites that allow you to do a modal quick product view, and then if you wish, click further in on that product to it's canonical page, for which the URL should definitely change, meaning the modal view should be using query params probably.

Add to this that query params are what's going to be used most of the time for appending search and filter data to a URL much like Zillow or other real estate sites for example, or google maps when the user zooms/pans the map and google does a shallow routing, or most online shopping sites where users do search and filter.

Sorry for long post!

@Antoine-lb
Copy link

@joehenry087 I don't really agree. While Instagram may have abused web standards, the resulting design is great for UX.

Once a URL can point to two totally different pages of a website […], user experience can suffer

The goal is to point to the same thing in different contexts. Any other use would be abusing this feature, but that is the responsibility of the developer.

[…] use a query param for the modal content? Then when someone sends the link, the recipient will see exactly what the sender saw - the main page behind with the modal content intended to be shared above - good UX 👍

There are some side effects of this approach:

  • You have to load two pages to achieve this (the "background site" that you probably don't care about and the modal).
  • What happens if the sender of the link infinitely scrolled till loading 3000 articles in the page before opening the modal? Should the recipient load those 3000 articles just to display "exactly what the sender saw"?
  • What if on desktop things open in a modal but on the phone they open in a new page?

Many teams have gone to great lengths to implement this feature because they know it is the best approach for the problem.

Personally, I would have implemented this method in multiple places if it were feasible and easy to maintain. That's why I'm excited for SvelteKit to take care of the heavy lifting.

@joehenry087
Copy link

joehenry087 commented Sep 12, 2023

** edit, I had a response to your points @Antoine-lb but I realized it's all off topic. My primary point, regardless of the use of shallow routing, is that query params are still a massively important part of web dev, more heavily relied upon than shallow routing I'd guess, which is relatively new, and to simply advocate to prioritize them over shallow routing. If there's more community support to prioritize shallow routing, then so be it.

@jer-0
Copy link

jer-0 commented Sep 12, 2023

Please release this feature mr. Rich🙏🙏. It's been 5 months, I can't wait any longer.

@Antoine-lb
Copy link

@joehenry087 I completely agree that query parameters should take priority over shallow routing. It's just more "web".

However, there's nothing stopping you from implementing this now - SvelteKit is well-equipped to handle it.

Shallow routing, on the other hand, can get messy if you try to do it on your own. That's why we're excited about it

@joehenry087
Copy link

@Antoine-lb do you have an example on how to implement the query parameters feature handy? In my initial testing I was finding it very difficult to do because pushing to history API would interfere with the Svelte navigation functionality. It was a couple weeks ago but if I recall it was something that was a complete blocker for me and I could find no way around, and then there were the same issues as described in issue #2673

@Antoine-lb
Copy link

Antoine-lb commented Sep 12, 2023

@joehenry087 I'm happy to discuss further but we are getting off-topic, you can email me:
[edited]

@Oreilles
Copy link
Contributor

Oreilles commented Sep 27, 2023

This would be a great addition to kit. It would be awesome if it was possible to implement this behavior with a data-sveltekit attribute so that it could be progressively enhanced (EDIT: shallow routing only makes sense with JS enabled) more concise. For exemple, instead of:

<script>
    import { page } from "$app/stores";
    import { pushState } from "$app/navigation";
    
    $: modal_opened = $page.state.modal_opened;
    
    function openModal(e) {
        const { href } = e.currentTarget;
        pushState({ modal_opened: !modal_opened }, href);
    }
</script>

<a href="." on:click|preventDefault={openModal}>
<Modal opened={modal_opened} />

We could write:

<a href="." data-sveltekit-shallow-pushstate={{ modal_opened: !$page.state.modal_opened }}>
<Modal opened={$page.state.modal_opened} />

@Rishab49 Rishab49 mentioned this pull request Oct 5, 2023
@aradalvand
Copy link
Contributor

aradalvand commented Dec 4, 2023

Is there anything serious blocking this? Or just the todos?

This would be an absolutely incredible feature. Can't wait.

@Rich-Harris
Copy link
Member Author

Closing in favour of #11307

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.