node-muneem / anumargak

Simple but Fastest Web Router in nodejs
Other
39 stars 5 forks source link

Handle URLs with query param and hash string both #15

Closed amitguptagwl closed 5 years ago

amitguptagwl commented 5 years ago

Currently, the code expects an URL either with query string or with hash. We have to modify it if both are present.

Eg 'https://user:pass@sub.host.com:8080/p/a/t/h?query=string#hash'

jgarciabengochea commented 5 years ago

Hello! @amitguptagwl Could I take this one on? I've looked through some of the code and I'm assuming this has to do with the urlSlice function in utils?

Some questions I have 1) Will the url string always have query first then a hash? 2) Should this handle multiple queries and or multiple hashes? (if multiple hashes are even a possibility) 3) Is there a specific test file I should use to write tests for this method? 4) do you have any more examples of base/ special/ edge cases of url strings this should handle?

Thank you!

amitguptagwl commented 5 years ago

@jgarciabengochea thanks for your attention.

? is call query params identifier # is called fragment identifier.

1) Yes 2) There can be multiple query params.

\this\is\sample?q1=val&q2=val3
\this\is\sample#q1=val&q2=val3
\this\is\sample?q1=val&q2=val3#q1=val&q2=val3
\this\is\sample?q1=val&q2=val3#q1=val#q2=val3 //# => q1=val#q2=val3

In general, multiple # are not allowed. So we can probably provide an option multipleFragmentIdentifier: true. If set to false then throw an error. But let's handle it as a separate issue.

jgarciabengochea commented 5 years ago

@amitguptagwl I have just submitted a pull request with a summary of my implementation. Thank you again for the opportunity!