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

fix(router): keep existing state when replacing #5302

Closed
wants to merge 1 commit into from

Conversation

posva
Copy link
Contributor

@posva posva commented Dec 18, 2020

What kind of change does this PR introduce?

This is a proposal rather than a fix. I would say that the state should be preserved when calling history.replaceState instead of being deleted. I came from #4905 but I don't remember anymore a case where the current solution would fail...

What is the current behavior?

What is the new behavior?

What steps did you take to test this? This is required before we can merge, make sure to test the flow you've updated.

  1. Step A
  2. Step B
  3. Step C

Checklist

  • Documentation
  • Testing
  • Ready to be merged
  • Added myself to contributors table

@lbogdan
Copy link
Contributor

lbogdan commented Dec 18, 2020

Build for latest commit 2adeb22 is at https://pr5302.build.csb.dev/s/new.

@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 2adeb22:

Sandbox Source
Notifications Test Configuration

@github-actions
Copy link

This PR has automatically been marked stale because there has been no activity in a while. Please leave a comment if the issue has not been resolved, or if it is not stale for any other reason. After 2 weeks, this issue will automatically be closed, unless a comment is made or the stale label is removed.

@github-actions github-actions bot added the stale label Mar 19, 2021
@posva
Copy link
Contributor Author

posva commented Mar 19, 2021

Check

@github-actions github-actions bot removed the stale label Mar 20, 2021
@danilowoz
Copy link
Member

Thanks for helping us clean up!

This PR does not seem to improve user or developer experience in a noticeable way, even fixing any bug. We are very sorry that you spent time on this, only to see us closing the PR. But we hope you understand our reasoning and do not feel discouraged to make other contributions 😄

@danilowoz danilowoz closed this May 11, 2021
@posva
Copy link
Contributor Author

posva commented May 11, 2021

Sure. I don't remember anymore but if this replaceState() was called for the window on the right of code sandbox, then it should be changed with the proposal as replaceState() and removing the current state will break client side routing that relies on the history state like Vue Router 4.

If it's about the codesandbox application itself, then it doesn't matter

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.

None yet

3 participants