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(create-mathcer): remove duplicated decodeURIComponent #3268

Closed
wants to merge 1 commit into from

Conversation

claudiolcastro
Copy link

@claudiolcastro claudiolcastro commented Jul 28, 2020

So, checking this issue #2725
I found that removing the decodeURIComponent from the matchRoute kinda resolve the problem, because the param already comes decoded.

I don't know if this is the proper way to fix this issue, but I will give some examples below that worked for me:

URL: /route-matching/params/%252520-working/bar

PS*: Check the foo param

Without the fix :
FAIL-route-matching_params_%252520-working_bar

After the change:
WORK-route-matching_params_%252520-working_bar

This example was also broking:
URL: /route-matching/params/100%25-working/bar"

FAIL-route-matching_params_100%25-working_bar

Closes #2725
Closes #3138

Copy link
Member

@posva posva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this! Let's add a few e2e tests to check this behavior in hash-mode.js and basic.js. One for initial navigation and a different one for a router-link navigation

You can check existing tests in those files as well

@mestevezdeister
Copy link
Contributor

Any new about this issue? Does it definitely fix the problem? I've tried applying the commit in my repository and it works for me. Is it going to be merge in the 3.x.x release? I've checked v3.4.3 and does not include the fix, the decoding problem still occurs.

Thank you very much!

@posva
Copy link
Member

posva commented Sep 10, 2020

It's missing tests if anybody is up to contribute!

@mestevezdeister
Copy link
Contributor

mestevezdeister commented Sep 15, 2020

I've made the same modification and add the mentioned e2e tests in #3323

@posva posva closed this in #3323 Sep 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants