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

Two "?"s added to url #37

Open
MartinSStewart opened this issue Oct 28, 2019 · 1 comment
Open

Two "?"s added to url #37

MartinSStewart opened this issue Oct 28, 2019 · 1 comment

Comments

@MartinSStewart
Copy link

I wanted to take an existing url and modify the query parameters. I noticed Url is just a record type so I figured the correct way to do this is

{ currentUrl
    | query =
        Url.Builder.toQuery [ Url.Builder.string "data" "test" ] |> Just
}
    |> Url.toString

but when I did that the result was http://localhost:3000/??data=test instead of http://localhost:3000/?data=test.

This seems to be a bug or at least unintuitive behavior and what I ended up doing to make it work was add String.dropLeft 1 to the toQuery output.

@tmohme
Copy link

tmohme commented Mar 18, 2020

Same problem (and same workaround) here.

Although you can't say that the behaviour of the toQuery function is wrong, it is a pitfall that the produced output is not a direct acceptable input for the query field of the Url record in the same package.

Just mentioning the exact requirements of the query field of the Url record and what toQuery actually produces in the docs would be immensely helpful to avoid this pitfall.

jerith666 added a commit to jerith666/elm-route-url that referenced this issue Jul 28, 2020
…cation

* adapt to the new Browser.application API in the following ways:

  * mirror the two-phase handling of URL changes in
    Browser.application's 'onUrlRequest' and 'onUrlChange' by
    bifurcating the RouterMsg variant of WrappedMsg into
    RouterMsgOnUrlChange and RouterMsgOnUrlRequestInternal variants.

  * add a slot to RouterModel to store the new Browser.Navigation.Key,
    and in the update function, use it to invoke
    Browser.Navigation.pushUrl in response to a urlRequestInternal.

  * create a new 'onExternalUrlRequest' function for the user to
    implement, since RouteUrl can handle internal requests for the
    user, but can't do anything sensible with external requests (as
    suggested by @basti1302).

  * eliminate the distinction between App and AppWithFlags, and all
    related duplication, as there is no variant of the new
    Browser.application that doesn't support flags.

* make UrlChange more strongly typed, mirroring the structure of the
  Url.Url record type from elm/url, and rework the way UrlChanges are
  converted to Cmds with a new 'apply : Url -> UrlChange -> Url'
  function.

* update all examples to work with the new API and 0.19 generally

  * include work-arounds for a couple of elm/url bugs
    (elm/url#37 and
    elm/url#17)

  * store the base path in ExampleViewer.Model to illustrate absolute
    path requirement of UrlChange

  * build the examples with '--debug' so users can get an idea for how
    they work under the hood

* update README

  * remove references to complementary packages that aren't compatible
    with 0.19 (which is all of them)

* remove the RouteUrl.Builder module and the use of the sporto/erl
  package, as this functionality is now largely provided by elm/url.

* remove the older RouteHash module, as it was only present to ease
  the transition from elm-route-hash to elm-route-url.  also remove
  example code illustrating its use.
jerith666 added a commit to jerith666/elm-route-url that referenced this issue Jul 28, 2020
…cation

* adapt to the new Browser.application API in the following ways:

  * mirror the two-phase handling of URL changes in
    Browser.application's 'onUrlRequest' and 'onUrlChange' by
    bifurcating the RouterMsg variant of WrappedMsg into
    RouterMsgOnUrlChange and RouterMsgOnUrlRequestInternal variants.

  * add a slot to RouterModel to store the new Browser.Navigation.Key,
    and in the update function, use it to invoke
    Browser.Navigation.pushUrl in response to a urlRequestInternal.

  * create a new 'onExternalUrlRequest' function for the user to
    implement, since RouteUrl can handle internal requests for the
    user, but can't do anything sensible with external requests (as
    suggested by @basti1302).

  * eliminate the distinction between App and AppWithFlags, and all
    related duplication, as there is no variant of the new
    Browser.application that doesn't support flags.

* make UrlChange more strongly typed, mirroring the structure of the
  Url.Url record type from elm/url, and rework the way UrlChanges are
  converted to Cmds with a new 'apply : Url -> UrlChange -> Url'
  function.

* update all examples to work with the new API and 0.19 generally

  * include work-arounds for a couple of elm/url bugs
    (elm/url#37 and
    elm/url#17)

  * store the base path in ExampleViewer.Model to illustrate absolute
    path requirement of UrlChange

  * build the examples with '--debug' so users can get an idea for how
    they work under the hood

* update README

  * remove references to complementary packages that aren't compatible
    with 0.19 (which is all of them)

* remove the RouteUrl.Builder module and the use of the sporto/erl
  package, as this functionality is now largely provided by elm/url.

* remove the older RouteHash module, as it was only present to ease
  the transition from elm-route-hash to elm-route-url.  also remove
  example code illustrating its use.
jerith666 added a commit to jerith666/elm-route-url that referenced this issue Jul 28, 2020
…cation

* adapt to the new Browser.application API in the following ways:

  * mirror the two-phase handling of URL changes in
    Browser.application's 'onUrlRequest' and 'onUrlChange' by
    bifurcating the RouterMsg variant of WrappedMsg into
    RouterMsgOnUrlChange and RouterMsgOnUrlRequestInternal variants.

  * add a slot to RouterModel to store the new Browser.Navigation.Key,
    and in the update function, use it to invoke
    Browser.Navigation.pushUrl in response to a urlRequestInternal.

  * create a new 'onExternalUrlRequest' function for the user to
    implement, since RouteUrl can handle internal requests for the
    user, but can't do anything sensible with external requests (as
    suggested by @basti1302).

  * eliminate the distinction between App and AppWithFlags, and all
    related duplication, as there is no variant of the new
    Browser.application that doesn't support flags.

* make UrlChange more strongly typed, mirroring the structure of the
  Url.Url record type from elm/url, and rework the way UrlChanges are
  converted to Cmds with a new 'apply : Url -> UrlChange -> Url'
  function.

* update all examples to work with the new API and 0.19 generally

  * include work-arounds for a couple of elm/url bugs
    (elm/url#37 and
    elm/url#17)

  * store the base path in ExampleViewer.Model to illustrate absolute
    path requirement of UrlChange

  * build the examples with '--debug' so users can get an idea for how
    they work under the hood

* update README

  * remove references to complementary packages that aren't compatible
    with 0.19 (which is all of them)

* remove the RouteUrl.Builder module and the use of the sporto/erl
  package, as this functionality is now largely provided by elm/url.

* remove the older RouteHash module, as it was only present to ease
  the transition from elm-route-hash to elm-route-url.  also remove
  example code illustrating its use.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants