kit icon indicating copy to clipboard operation
kit copied to clipboard

fix: restore `history.state` on refresh

Open yahao87 opened this issue 1 year ago • 6 comments

fixes? https://github.com/sveltejs/kit/issues/11956 fixes https://github.com/sveltejs/kit/issues/9868

Fix the issue with history.state resetting on refresh


Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • [x] 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:.

Edits

  • [x] Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed.

yahao87 avatar Jun 28 '24 12:06 yahao87

⚠️ No Changeset found

Latest commit: 79de4276fb259d26aa18ba6d08c73049a845f4b6

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

changeset-bot[bot] avatar Jun 28 '24 12:06 changeset-bot[bot]

this seems to be an area that's difficult to get right, so I'd really want to have a test for this one

benmccann avatar Jun 28 '24 20:06 benmccann

The maintainers probably haven't discussed this yet, but I think this should be optional as Rich mentioned in the issue. Also, I don't think this is the correct fix. This would only work if SSR is disabled.

PatrickG avatar Jun 30 '24 17:06 PatrickG

The maintainers probably haven't discussed this yet, but I think this should be optional as Rich mentioned in the issue. Also, I don't think this is the correct fix. This would only work if SSR is disabled.

I don't understand. Why should the framework's default behavior be to block the implementation of basic browser functionality? If a user is building a complex multi-page signup process, they can use history.state to store personal information within the history to preserve it even after a refresh without relying on a store. This history.state can store not only simple objects but also data like files selected through a form. Blocking this forces the use of localstorage.

It's natural that this doesn't work in SSR since it's a browser feature. Isn't it up to the developers using this framework to handle such implementations? The current implementation in Kit2 blocks this, taking away the user's choice.

Our service has been unable to upgrade to Kit2 since last year due to this issue.

yahao87 avatar Jul 01 '24 00:07 yahao87

The current behavior of SvelteKit 2, which resets history.state on refresh, poses significant challenges for developers. When building complex multi-page signup processes, using history.state to preserve personal information even after a refresh is extremely useful. This allows developers to store data like files selected through a form without relying on local storage.

Blocking basic browser functionality imposes unnecessary constraints on developers. While it is understandable that history.state would not work in SSR (Server-Side Rendering) environments, limiting its use on the client side is inappropriate.

Moreover, the current implementation in SvelteKit 2 lacks the necessary flexibility. Instead of enforcing specific functionalities, the framework should allow developers to make their own choices. This flexibility is crucial for supporting a wide range of use cases. The inability of certain services to upgrade to SvelteKit 2 due to this restriction highlights the real problems this limitation can cause.

Overall, the current implementation of SvelteKit 2, which restricts the use of history.state, imposes significant constraints on developers and limits support for various use cases. Giving developers the flexibility to choose how they manage state is the right approach. A framework should not block basic functionality but rather empower developers to make decisions based on their specific needs.

maj0rika avatar Jul 01 '24 02:07 maj0rika

The reason I mentioned SSR is because the solution for the linked issue #11956 (which is about $page.state not history.state) should work with SSR enabled as well. I guess this PR is supposed to be a fix for #9868 (which IMO should be fixed of course).

PatrickG avatar Jul 01 '24 06:07 PatrickG