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

beforeRouteUpdate fires when it must not if is used with cyrillic symbols #2614

Closed
smellyshovel opened this issue Feb 12, 2019 · 7 comments
Closed

Comments

@smellyshovel
Copy link

Version

3.0.2

Reproduction link

https://codesandbox.io/s/rmnq4605oo

Steps to reproduce

  1. Add the following to the path (right in the browser preview): "fm/русский"

    The address bar should now look something like this: https://rmnq4605oo.codesandbox.io/fm/русский

  2. Click enter so to pretend that it's router's initial state (without any previous navigation via vue-router (because the page actually gets reloaded and vue-router isn't involved in this))

  3. Open the console (F12)

  4. Double-click the "русский" word

What is expected?

The beforeRouteUpdate must not be fired.

What is actually happening?

It actually gets fired. As far as I understand it happens because "русский" gets substituted with "%D1%80%D1%83%D1%81%D1%81%D0%BA%D0%B8%D0%B9" as you can see from the console output.


Don't know if it's only due to cyrillic symbols, but I'm pretty sure that it's expected for all the non-ascii charecters.

@smellyshovel
Copy link
Author

Don't know if it's only due to cyrillic symbols, but I'm pretty sure that it's expected for all the non-ascii charecters.

Seems like i'm correct. Check the same for path/with/slashes (see the reproduction link). All the slashes (/) get substituted with %2F which also triggers the beforeRouteUpdate.

@posva
Copy link
Member

posva commented Feb 13, 2019

I tested locally with Chrome and Safari (code sandbox wasn't loading) and couldn't reproduce.
Keep in mind unicode characters are not supported in all browsers. See #838

@posva posva closed this as completed Feb 13, 2019
@smellyshovel
Copy link
Author

smellyshovel commented Feb 13, 2019

@posva I can reproduce both locally (Chrome 72) and at code sandbox.

Keep in mind unicode characters are not supported in all browsers. See #838

OK, I can see that shouldn't use non-unicode characters for addresses. But what about slashes? My route may contain multiple slahes which are initially passed correct (as /), but after navigating with vue-router these are sabstituted with %2F which triggers beforeRouteUpdated as well.

I don't believe that it's not a bug.

You can test this at code sandbox as well, just navigate to "fm/path/with/slashes" instead of "fm/русский".

Thank you for your attention.


Adding more details since you can't load code sandbox

The route definition:

path: "/fm/:folderLink*",
name: "fm",
component: FileManager,
props: ({ params }) => ({ folderLink: params.folderLink || "" })

Initial route's path (after page refresh) looks like: русский.
Address bar looks like: https://rmnq4605oo.codesandbox.io/fm/русский

What's executed next:

router.push({ name: "fm", params: { folderLink: "fm/русский" } });

Now the route's path is: /fm/%D1%80%D1%83%D1%81%D1%81%D0%BA%D0%B8%D0%B9
But the address in the address bar hasn't change at all (!): https://rmnq4605oo.codesandbox.io/fm/русский

So the beforeRouteUpdate must not trigger. But it does!

All the abovestated applies for /fm/path/with/slashes as well.

@posva
Copy link
Member

posva commented Feb 13, 2019

slashes are different because they are encoded so they are matched as parameters and not separators
Anyway, I will look back at this when I tackle the problem about unicode characters

@smellyshovel
Copy link
Author

smellyshovel commented Feb 13, 2019

@posva OK, looking forward for it. Would you please notice me somehow when this is fixed? Thank you in advance.

By the way, I found another drawback of the current implementation.

Neither of .router-link-active nor .router-link-exact-active aren't initially added to a link pointing to cyrillic addresses. The link is only highlighted after navigating via vue-router. E.g.:

<router-link :to="{ name: "fm", params: { folderLink: "русский" } }">Русский</router-link>
...

<style>
.router-link-exact-active, .router-link-active {
    color: green;
}
</style>

The link initially isn't green. It only becomes green after you click on it.

However plain-english links (even with slashes) work fine.

<router-link :to="{ name: "fm", params: { folderLink: "no/special/chars/but/with/slashes" } }">Is green by default</router-link>

Din't know if it was worth it to open a new issue for it, so decided to post it here. However I could formatte it to be a separate issue if you tell me so.

Thanks.

@posva
Copy link
Member

posva commented Feb 13, 2019

I won't be able to notify you, I will just not remember... subscribe to the issue I mentioned up there, that's also tracking for active links. Cheers

@smellyshovel
Copy link
Author

smellyshovel commented Feb 13, 2019

Sorry for posting to the closed issue but I believe that someone might find this useful.

Long story short I guess I found the solution to my (and probably many more) problems.

Here's the Code Sandbox link: https://codesandbox.io/s/0oqjpxr520.

The key part is the following one

router.beforeEach((to, from, next) => {
    let newPath = decodeURIComponent(to.path);

    if (to.fullPath !== newPath) {
        next({ path: newPath, replace: true });
    } else {
        next();
    }
});

What we do here is basically just manually decoding any URI and replace the path the Vue Router wants to navigate to with the decoded one.

Works fine for me, solves issues such as: this issue's main topic, issues with params that contain slashes. Though it doesn't solve the issue with .router-link-active, .router-link-exact-active for me (see above).

I know, my solution looks a bit naive, but it works and what's most important - it works correct (at least as far as I can see).

So, if this post helped someone - you're welcome 😃

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

2 participants