AlexxNB / tinro

Highly declarative, tiny, dependency free router for Svelte's web applications.
MIT License
675 stars 30 forks source link

Add Replace option for Redirect Route #56

Closed wtachau closed 3 years ago

wtachau commented 3 years ago

The current redirect logic uses history.pushState, which in one of our uses cases means the browser "back" button doesn't work correctly. If we navigate from A to B, which redirects to C via pushState, then clicking "back" will take us back to B, which promptly redirects to C again. Instead, history.replaceState will remove B from the stack, and correctly take us from C to A.

Example:

 <Route path="/" redirect={'/foo'} replace>

This is a great library -- let me know what changes you would like to see in the PR!

AlexxNB commented 3 years ago

I think replace property doesn't need. It should be by default, when redirects happening.

AlexxNB commented 3 years ago

And what we can do in hash mode?

wtachau commented 3 years ago

Are you suggesting we make pushState the default for redirects?

And, sorry, what do you mean by hash mode?

wtachau commented 3 years ago

Ah, the hash mode of navigation. I haven't tried that mode, but it looks like nothing would need to be done for that method, right? https://github.com/AlexxNB/tinro/blob/master/src/location.js#L51

AlexxNB commented 3 years ago

I need check it, but as I remember replacing hash is also cause history entries in back button. We need same behaviour in both cases - history and hash modes.

And I dont like, you added new method to the router object. I think, better to add second optional parameter to the router.goto method, which will set replace or not the URL

wtachau commented 3 years ago

@AlexxNB I added an optional parameter to router.goto 👍

AlexxNB commented 3 years ago

Cool. I wanna do some refactoring , but seems you don't allowed code editing by maintainer in your fork.

AlexxNB commented 3 years ago

Never mind. I'll merge and edit in master repo.

AlexxNB commented 3 years ago

Thank you, for your PR.