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

History state is overwritten to {} during navigation since next.208 #3236

Closed
gustavopch opened this issue Jan 7, 2022 · 6 comments
Closed
Labels
bug Something isn't working p3-edge-case SvelteKit cannot be used in an uncommon way router
Milestone

Comments

@gustavopch
Copy link

Describe the bug

Since @sveltejs/kit@1.0.0-next.208, goto()'s state parameter stopped working. This line is overwriting the state to {} whenever navigation occurs:

history.replaceState({}, '', info.url);

This is the commit that introduced the bug: d17f459.

Reproduction

  • Open the repro: https://stackblitz.com/edit/sveltekit-xfxpb3
  • Click the button "Goto with state".
  • It will take you to another route where the History state will be visible.
  • You'll see the state will be empty. It should be what was passed to goto().

Logs

No response

System Info

System:
    OS: macOS 12.0.1
    CPU: (8) arm64 Apple M1
    Memory: 112.81 MB / 8.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 16.13.1 - ~/.asdf/installs/nodejs/16.13.1/bin/node
    Yarn: 1.22.17 - ~/.asdf/installs/nodejs/16.13.1/.npm/bin/yarn
    npm: 8.2.0 - ~/.asdf/installs/nodejs/16.13.1/.npm/bin/npm
    Watchman: 2021.11.15.00 - /opt/homebrew/bin/watchman
  Browsers:
    Chrome: 97.0.4692.71
    Safari: 15.1
  npmPackages:
    @sveltejs/adapter-auto: 1.0.0-next.9 => 1.0.0-next.9 
    @sveltejs/kit: 1.0.0-next.208 => 1.0.0-next.208 
    svelte: 3.44.3 => 3.44.3

Severity

blocking all usage of SvelteKit

Additional Information

No response

@bluwy bluwy added bug Something isn't working p3-edge-case SvelteKit cannot be used in an uncommon way router labels Jan 8, 2022
@benmccann
Copy link
Member

@gustavopch thanks for the investigation! care to send a PR?

@gustavopch
Copy link
Author

@benmccann This would fix the problem, but I'm not sure if it might break some other behavior. What do you think?

- history.replaceState({}, '', info.url);
+ history.replaceState(history.state, '', info.url);

@zommerberg
Copy link
Contributor

This won't be needed once #3241 is merged

@benmccann benmccann added this to the 1.0 milestone Jan 9, 2022
@benmccann benmccann linked a pull request Jan 12, 2022 that will close this issue
5 tasks
@benmccann
Copy link
Member

Rich sent #3312 to remove this feature, which would in effect close this issue.

Regarding the severity of "blocking all usage of SvelteKit" - I don't think there's anything you could do with this API that you couldn't do with local storage? But if there's a usercase you have for this feature, you may want to chime in on #1641 to help make it clearer what it's being used for

@gustavopch
Copy link
Author

gustavopch commented Jan 12, 2022

@benmccann Local storage, session storage, and history state are not interchangeable. They have different lifecycles and serve different needs.

  • Local storage: data persists when you close the tab.
  • Session storage: data persists when you navigate to other routes, but not when you close the tab.
  • History state: data is linked to that particular history entry.

Whenever you have data that only makes sense for a specific route, history state is the way to go.

Actual use case: history-controlled modals

<script>
  import { goto } from '$app/navigation'
  import { historyState } from './store'
</script>

<button on:click={() => goto('', { state: { modal: 'first' } })}>Open first modal</button>

<Modal open={$historyState.modal === 'first'}>
  <button on:click={() => goto('', { state: { modal: 'second' } })}>Open second modal</button>
</Modal>

<Modal open={$historyState.modal === 'second'}>
  <button on:click={() => history.go(-2)}>Close</button>
</Modal>

I have historyState which is a store that listens to history.state. To open the modal, I add a history entry with its state determining that the modal should be open. If the user presses the back button, the modal will automatically close. It's as simple as it can get. Using local storage or session storage would require me to basically keep them in sync with the history.

That said, I think there would be no problem if I had to call history.pushState directly instead of using goto. 🤔

@gustavopch
Copy link
Author

I'm closing since it was fixed in e3cb69b.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p3-edge-case SvelteKit cannot be used in an uncommon way router
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants