search

solidjs / solid-router

A universal router for Solid inspired by Ember and React Router
MIT License
1.13k stars 143 forks source link

Unable to use encoded forward slash as route param #220

Closed goenning closed 1 year ago

goenning commented 1 year ago

Describe the bug

I have a route path like /:name/view and then the name might actually contain a forward slash, so i encode it with encodeURIComponent, but solid-router doesn't route it correct, it goes straight to 404.

Your Example Website or App

https://stackblitz.com/edit/solidjs-template-15p2rm

Steps to Reproduce the Bug or Issue

  1. Go to https://stackblitz.com/edit/solidjs-template-15p2rm
  2. Start it
  3. Navigate to /hello-world/view and it'll work
  4. Navigate to /hello%2Fworld/view and you'll see a 404 page

Expected behavior

I expect the url /hello%2Fworld/view to match the /:name/view route path.

Screenshots or Videos

No response

Platform

Additional context

No response

Brendan-csel commented 1 year ago

I've got a commit that fixes this in a custom build of the router.

This stackblitz link is using our "patched" router to demonstrate the solution. Does that work for you?

I just haven't submitted this as a PR here as I'm not sure if it would be breaking for others.

image

goenning commented 1 year ago

Yeah, your package works for me. But there is a breaking change though, on the official package if I use encodeURIComponent, I don't need to decode on the useParams.

It seems like with your package I need to decode it when reading it.

Brendan-csel commented 1 year ago

Yeah this fix is basically reverting more of the change introduced on May 18th in this commit (of which some has already been reverted).

Aside from solving your issue, my feeling is the router shouldn't be decoding things it didn't encode in the first place. Like if the application code is encoding something it in not unreasonable to expect to have to decode it when used.

@ryansolid Let me know if you'd like me to submit the commit linked above as a PR (or just grab it if you want to).

ryansolid commented 1 year ago

I agree. We shouldn't be decoding things we didn't encode in the first place.

Looking back at the original issue(https://github.com/solidjs/solid-router/issues/112), I think expecting spaces in the URL to work is strange anyway. I haven't used other modern solutions but like I'd expect if I chose to put a space in there I'd need to do the %20 myself.

Brendan-csel commented 1 year ago

Pull request submitted. https://github.com/solidjs/solid-router/pull/222