Servant.Links.addQueryParam not exported?

[saurabhnanda]

I was trying to write a ToLink instance for a custom combinator called AllQueryParams (that captures all query parameters). Unfortunately, it seems almost impossible without resorting to type-level magic, because neither are the constructors of Link exported, nor is addQueryParams exported.

Is there an obvious way of doing this, which I might be missing?

Here's my (non-working) code snippet:

data AllQueryParams

instance (HasServer api context) => HasServer (AllQueryParams :> api) context where
  type ServerT (AllQueryParams :> api) m = ([(BS.ByteString, Maybe BS.ByteString)] -> ServerT api m)

  hoistServerWithContext _ pc nt s = hoistServerWithContext (Proxy :: Proxy api) pc nt . s

  route _ context subServer = route (Proxy :: Proxy api) context delayed
    where
      delayed = passToServer subServer queryString

instance (HasLink sub) => HasLink (AllQueryParams :> sub) where
  type MkLink (AllQueryParams :> sub) a = ([(BS.ByteString, Maybe BS.ByteString)] -> MkLink sub a)

  toLink toL _ lnk qparams = toLink toL (Proxy :: Proxy sub) $
                             DL.foldl' (\l (k, v) -> addQueryParam (SingleParam (toS k) (toS $ fromMaybe "" v)) l) lnk qparams

[phadej]

I don't see a reason for it not to be exported. Please make a PR.

[saurabhnanda]

Any chance that it could make the concept of safe links unsafe?

[phadej]

There's a balance between being extensible, vs. preventing from programming errors. Maybe addSegment and addQueryParam should be exported from Servant.Links.Internal

@alpmestan opinions?

[alpmestan]

We've usually tried to keep things extensible whenever/wherever appropriate, and I think we definitely want to allow users to hook their own combinators in the link making machinery.

I don't think that really makes our links less type safe. The only way to get them wrong is to make a mistake in an instance or do something bad with them, explicitly, after the link making function handed us correct ones. In the first case, well, it's easy to spot with some tests, and the code for those instances is fairly easy to follow. In the second, well it should be obvious where the problem comes from.

Finally, if people have to get those functions from Servant.Links.Internal, then they know they're exposed to the guts and have to be careful not to introduce errors themselves, I think.

=> I'm all for exporting those utilities from somewhere, especially if that somewhere makes it obvious that it's not the public API that users who just want to produce links need.

[poscat0x04]

I just bumped into this problem as well. IMO HasLink is too restrictive right now.

[alpmestan]

A PR is still welcome :-)

[saurabhnanda]

I hit this again, while solving #1784 for my own codebase... I think I'll raise a PR now if no one has any objections 😄