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

router push gives DOMException: Failed to execute 'replace' on 'Location': 'http://localhost:8080undefined' is not a valid URL #366

Closed
leopsidom opened this issue Jul 12, 2020 · 15 comments

Comments

@leopsidom
Copy link

Using router.push(path) but get an error as in the title. When looking at the debugger, it fails at the changeLocation line:

    function push(to, data) {
        const currentState = assign({}, history.state, {
            forward: to,
            scroll: computeScrollPosition(),
        });
        changeLocation(currentState.current, currentState, true);    // this is the line that fails

I can see in the debugger that to has the correct value but currentState.current is undefined. What could be the issue?

@vue-bot
Copy link
Collaborator

vue-bot commented Jul 12, 2020

Hello, thank you for taking time filling this issue!

However, we kindly ask you to use our Issue Helper when creating new issues, in order to ensure every issue provides the necessary information for us to investigate. This explains why your issue has been automatically closed by me (your robot friend!).

I hope to see your helper-created issue very soon!

@vue-bot vue-bot closed this as completed Jul 12, 2020
@frandiox
Copy link
Contributor

@leopsidom Did you figure it out? I'm getting the same error when calling window.history.replaceState(...) right before router.push(...) 🤔

@posva
Copy link
Member

posva commented Aug 21, 2020

If you manage to reproduce, please open a new issue! Calling replaceState (or push) without passing the current state will fail. Do this:

history.replaceState(history.state, '', ...)

@frandiox
Copy link
Contributor

@posva Thanks a lot, that did the trick. I was doing replaceState({}, '', ...) before since it was working in the previous router version. ありがとう!

@posva
Copy link
Member

posva commented Aug 21, 2020

I'm interested in the use cases for manually calling history.push|replace because right now I really don't see where to document that as in practice one shouldn't be manually changing the URL when using a client-side router

@frandiox
Copy link
Contributor

frandiox commented Aug 21, 2020

@posva I think a common use case is saving form values or pagination info in the history (as query string) so you can go back, refresh or link to that page while keeping the form state.

I remember checking this in the previous router but could not find a way to do it properly so I had to modify the history manually. Not sure if there's a way to do it in the current version of the router, I haven't tried since I'm just upgrading the existing code to Vue 3.

@posva
Copy link
Member

posva commented Aug 21, 2020

Even though it's not documented, you can now pass a state property when pushing or replacing that will be passed to history.pushState. It's likely to change though, it could become a different function since sometimes you just want to modify the current history entry's state.

You can natively save state with history.replaceState({ ...history.state, ...myOwnState }, '') and it should not break the router.

I thought the form state was automatically saved by browsers 😄

I think, being able to pass state to history.state will go through an RFC at some point to make this behavior more explicit

@leopsidom
Copy link
Author

For me, this issue still exists. Basically router.push(...) fails when the website is visited for the very first time. The easy way to reproduce is when I visit the site in the incognito mode.

@posva
Copy link
Member

posva commented Aug 21, 2020

@leopsidom I won't be able to help if you don't provide a boiled down reproduction 🙂
In the issue helper, you can find a starting boilerplate to build a reproduction. An HTML file content also works

@leopsidom
Copy link
Author

Hmm, I was not able to reproduce it from scratch. Maybe it's a vue cli issue. Previously I did vue add vue-next to experiment with vue 3.

@leopsidom
Copy link
Author

leopsidom commented Aug 21, 2020

Okay, I think I got why this is happening. This happens when my website is redirected. For instance, I use a third party authentication service where I'm redirected from domain A to domain B to do authentication and after login, I'm redirected back to domain A. Here if I don't refresh the page and simply do a router.push(...), I will get the above issue. Not sure how to reproduce this issue pretty quickly though.

@leopsidom
Copy link
Author

Finally figure out the issue, I was using aws-amplify for my authentication. And the package will rewrite my windows history after auth here: https://github.com/aws-amplify/amplify-js/blob/main/packages/auth/src/Auth.ts#L1940. This line seems problematic from your comment above. While I am having a solution now, I wonder from vue-router perspective, whether it is better to handle these situations more gracefully ?

@posva
Copy link
Member

posva commented Aug 22, 2020

@leopsidom that's a bug on the library, they should keep the current state at https://github.com/aws-amplify/amplify-js/blob/main/packages/auth/src/Auth.ts#L1942

window.history.replaceState(
  window.history.state,
  '',
  (this._config.oauth as AwsCognitoOAuthOpts).redirectSignIn
);

Even if a gracefully handle it, it will break other features of Vue Router and create inconsistent situations. In other words, it will break elsewhere. I've been testing different interactions and they become inconsistent. I will still put up a PR because it's better than having an error. But I think, the best would be to have a warn

@alicemw
Copy link

alicemw commented Jul 1, 2021

why this problem is still confused me

@Viterbo
Copy link

Viterbo commented Jan 30, 2023

Workaround:

if you use history.pushState(...) directly alone with Vue router navigation you can avoid this problem by adding the current property yourself like this:

const new_state = {
   whatever: 'this is your data',
   current: '/'   // this is the workaround
}

window.history.pushState(new_state, '', '/next/path');

What was the problems?
I used history.pushState(...) to keep the navigation state on the URL but this way of changing URL does not use the Vue router which sets for itself the current property every time. If you use history.pushState(...) directly it does not fail immediately but it does not set the current property. So it fails the next time you try to use the normal Vue router.

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

No branches or pull requests

6 participants