Zaid-Ajaj / Feliz.Router

A router component for React and Elmish that is focused, powerful and extremely easy to use.
MIT License
78 stars 16 forks source link

How to handle query parameters? #26

Closed olivercoad closed 4 years ago

olivercoad commented 4 years ago

Hi, thanks for the quick fix for path mode. It works well now.

Just wondering what is the best way of handing query parameters?

The docs show how to use pattern matching, but it doesn't allow query parameters to be re-ordered and makes it difficult for handing combinations where parameters may be missing. Even with only two optional parameters it becomes quick verbose to handle properly.

let fromValue, toValue =
    match segments =
    | [ Route.Query [ "from_value", fromValue; "to_value", toValue ] ] -> Some fromValue, Some toValue
    | [ Route.Query [ "to_value", toValue; "from_value", fromValue ] ] -> Some fromValue, Some toValue
    | [ Route.Query [ "to_value", toValue ] ] -> None, Some toValue
    | [ Route.Query [ "from_value", fromValue ] ] -> Some fromValue, None

Furthermore, even if you don't use query parameters, routing could break if unexpected query parameters are present such as facebook's fbclid. This particularly seems like it would be a big problem, right?

Zaid-Ajaj commented 4 years ago

This particularly seems like it would be a big problem, right?

Not really, it is actually quite simple. Instead of matching against exact parameters, you can search for the parameters you are interested in:

type Range = { From: string option; To: string option } 

let find key parameters = 
  parameters
  |> List.tryFind (fun (name, value) -> name = key)
  |> Option.map snd 

let findRange segments = 
  match segments with 
  | [ Router.Query parameters ] -> 
     // here we have all the parameters
     let range = { 
         From = find "from_value" parameters
         To = find "to_value" parameters 
     }

     Some range

  | _ -> 
    None 
olivercoad commented 4 years ago

Oh of course! That's obvious in hindsight.

But still, what about routes that don't expect any query parameters? In order to not break when facebook adds fbclid, you'd still have to duplicate the match to throw away the query parameters.

match segments with
| [ ]
| [ Route.Query _ ] //redundant
    -> "home page"
| [ "users"; Route.Int userId ]
| [ "users"; Route.Int userId; Route.Query _ ]
    -> sprintf "User %i page" userId
| _ -> "404"
Zaid-Ajaj commented 4 years ago

@olivercoad It is still the exact same principle: start with searching. Concretely, you can first search for the query segment separately and then the rest of the non-query segments:

let findQuery (segments: string list) : (string * string) list = 
   segments
   |> List.choose (function 
       | Route.Query parameters -> Some parameters
       | _ -> None
   )
   |> List.tryHead 
   |> function 
        | Some parameters -> parameters
        | None -> [ ]

// URL segments without the query string parameters
let findNonQuerySegments (segments: string list) : string list = 
   segments
   |> List.filter (function 
         | Route.Query parameters -> false 
         | _ -> true
    )

Now you can easily deal with segments-only and then optionally deal with the query parameters if you want. The way makes for very flexible API and you can structure your route matching however you want :smile:

olivercoad commented 4 years ago

Yeah that would do it. Thanks for your help :)

Though it does kinda seem wrong for the library to join the pathname to the query only for the consuming code to have to separate them again. To be clear, the AppPath.fs demo just displays "Not Found" when there is an extra query parameter.

And one of the main reasons to use routing is to be able to save and share links, and many sites add extra parameters to links - so handling them isn't really optional (in path mode).

I ended up defining my own getCurrentUrl function:

let getCurrentUrl _ =
    let pathName = Browser.Dom.window.location.pathname
    let queryString = Browser.Dom.window.location.search

    let urlSegments = Router.urlSegments pathName RouteMode.Path
    let urlParams =
        (Router.createUrlSearchParams queryString).entries()
        |> Seq.map (fun arr -> (arr.[0], arr.[1]))
        |> Map.ofSeq
    urlSegments, urlParams

Would you accept a pull request for something like this?

Zaid-Ajaj commented 4 years ago

I would rather not add too many functions that do separate small things, if it is easy to do from user code like adding two small functions to achieve what you need there, then I would call it a day ;)

Zaid-Ajaj commented 4 years ago

I believe this issue is resolved

olivercoad commented 4 years ago

Yeah I guess the original query is resolved, though could probably do with additions to the docs.

When I get time I might open a separate issue for the part about pathname being joined with queryString and propose some possible changes.

Zaid-Ajaj commented 4 years ago

Yeah docs on how to handle things differently as needed would be great!