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

Was automatic decoding a mistake? #516

Closed
pshrmn opened this issue Aug 28, 2017 · 1 comment
Closed

Was automatic decoding a mistake? #516

pshrmn opened this issue Aug 28, 2017 · 1 comment

Comments

@pshrmn
Copy link
Contributor

pshrmn commented Aug 28, 2017

Since there have been issues, I thought that it might be appropriate to discuss the pros and cons of automatic pathname decoding in history.

Pros

  • Consistent location objects. If history always decodes the pathname, then location object's will always have a decoded pathname. If history does not do this, then locations created from window.location will have an encoded pathname, while locations created from the user will be encoded/unencoded based on what the user provides.
  • Unencoded pathnames make route matching easier (for React Router).

Cons

Edit: Deleted some stuff here, nothing particularly important.

Perhaps I'm just overthinking this, but whenever I see issues with the current implementation, it makes me think I jumped the gun when I implemented the automatic decoding. It definitely seems like a good idea, but text encoding is difficult/annoying to deal with and I think that it might be better to have consistent behavior than convenience with some edge cases.

@pshrmn
Copy link
Contributor Author

pshrmn commented Aug 29, 2017

I just took a look at what Django does, and they also decode pathnames (to the extent that a request for /test%2fing will be matched by r"^test/ing$", which I'm not entirely convinced is the desirable). They also reference RFC 3987, which I should probably take a look at (not sure how relevant it is for history).

I think that the important thing is that the pathname pushed to the browser is what the user provides us. That should be doable while still automatically decoding the pathname. I'll look into a possible PR for this behavior sometime soon.

@pshrmn pshrmn closed this as completed Aug 29, 2017
@lock lock bot locked as resolved and limited conversation to collaborators Jun 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant