elmish / urlParser

Turn URLs into statically-typed data
https://elmish.github.io/urlParser
Other
7 stars 5 forks source link

Fable 4.0 breaks parseParams #25

Closed forki closed 11 months ago

forki commented 1 year ago

Before the Fable 4.0 update we did:

let urlParser location = parsePath pageParser location

but now we have to do

let urlParser (location:Browser.Types.Location) =
    parse pageParser location.pathname (parseParams (location.search.Replace("?","")))

It looks like the ? is no longer handled automatically.

/cc @et1975 @alfonsogarciacaro @MangelMaxime @tforkmann

alfonsogarciacaro commented 1 year ago

That's strange. Did you only update Fable or also Elmish? Would it be possible to print the generated code for Fable 3 and Fable 4 for urlParser?

forki commented 1 year ago

for the record: I don't think it's a fable compiler bug.

et1975 commented 1 year ago

This library doesn't work on Browser.Types.Location because it was meant to be cross-platform from the get-go. The original version baked into Fable.Elmish.Browser prior to the split did have that dependency though, so maybe that's the change you're observing.

mlaily commented 11 months ago

Hello!

I don't have any idea about the history of these projects, but I confirm there is currently an incompatibility between them:

parsePath and parseHash from Fable.Elmish.Browser (https://github.com/elmish/browser/blob/v4.x/src/parser.fs#L16-L33) do not remove the "?" from the query param string, and pass it directly to parseParams from Fable.Elmish.UrlParser (https://github.com/elmish/urlParser/blob/master/src/parser.fs#L356)

The issue is that this parseParams expects the string to not contain the initial "?" (as hinted by the fact the parseUrl from Fable.Elmish.UrlParser does remove it before using parseParams https://github.com/elmish/urlParser/blob/master/src/parser.fs#L368-L375)

I'm not sure what's the best approach to fix it, but I see two options:

This reimplementation of parseParams in Fable.Elmish.Browser could take inspiration from Feliz.Router and use the native URLSearchParams query param parser (that expects an initial "?", so it would fit naturally) https://github.com/Zaid-Ajaj/Feliz.Router/blob/master/src/Router.fs#L141-L142


Based on the above, here is what I came up with (just for parseHash, but it should work the same for parsePath):

    type private IUrlSearchParameters =
        abstract entries : unit -> seq<string array>

    [<Emit("new URLSearchParams($0)")>]
    let private createUrlSearchParams (_queryString: string) : IUrlSearchParameters = jsNative

    let parseHash (parser: Parser<_,_>) (location: Browser.Types.Location) =
        let parseParams queryString =
            try
                let urlParams = createUrlSearchParams queryString
                Map [ for entry in urlParams.entries() -> entry[0], entry[1] ]
            with
            | _ -> Map.empty

        let hash, search =
            let hash =
                if location.hash.Length > 1 then location.hash.Substring(1)
                else ""
            if hash.Contains("?") then
                let justHash = hash.Substring(0, hash.IndexOf("?"))
                justHash, hash.Substring(justHash.Length)
            else
                hash, "?"

        parse parser hash (parseParams search)

It seems to work well as a workaround too, until this is fixed.

et1975 commented 11 months ago

Released fix in 1.0.2, please give it a spin.

mlaily commented 11 months ago

Released fix in 1.0.2, please give it a spin.

What about adding unit tests instead? :)

https://github.com/elmish/browser/pull/69