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

Is DeepQuery missing a HasLink instance? #1784

Open
saurabhnanda opened this issue Sep 22, 2024 · 3 comments
Open

Is DeepQuery missing a HasLink instance? #1784

saurabhnanda opened this issue Sep 22, 2024 · 3 comments

Comments

@saurabhnanda
Copy link

Upgraded to servant-server-0.20.2 to specifically use the new DeepQuery feature, however, got the following compilation error upon using it:

/home/vl/vacationlabs/haskell/src/Plugin/ItinerarySegments/Links.hs:13:14-38: error:
    * There is no instance for HasLink (DeepQuery
                                          "search" Models.ItinerarySegment.SearchOpts :> ...)
    * In the expression:
          allFieldLinks' linkToText :: Routes (AsLink Text)
      In a pattern binding:
        Routes {..} = allFieldLinks' linkToText :: Routes (AsLink Text)
   |
13 | Routes{..} = allFieldLinks' linkToText :: Routes (AsLink Text)
   |              ^^^^^^^^^^^^^^^^^^^^^^^^^
@tchoutri
Copy link
Contributor

cc @divarvel

@saurabhnanda
Copy link
Author

@tchoutri @divarvel does this look good to raise a PR (except the toS stuff, of course):

instance (KnownSymbol sym, ToDeepQuery record, HasLink sub) => HasLink (DeepQuery sym record :> sub) where
  type MkLink (DeepQuery sym record :> sub) a =
    record -> MkLink sub a

  -- toLink :: (Link -> a) -> Proxy endpoint -> Link -> MkLink endpoint a
  toLink toA _ lnk record =
    toLink toA (Proxy @sub) $ addParams lnk
    where
      k :: Text
      k = toS $ symbolVal (Proxy @sym)

      mkSingleParam x = 
        let (a, b) = generateDeepParam k x
        in SingleParam (toS a) (toS $ escapeURIString isUnreserved $ toS $ fromMaybe "" b)

      addParams lnk = 
        DL.foldl' (\memo x -> addQueryParam (mkSingleParam x) memo) lnk $ 
        toDeepQuery record

Also, why does ToDeepQuery deal with Text when inserting the results into the actual Link would force one to convert them to String?

@divarvel
Copy link
Contributor

ah yes, the missing HasLink instance is an overview indeed.

The PR makes the choice to use Text everywhere instead of String in alignment with Network.HTTP.Types which provides QueryText, and more generally to avoid using strings as much as possible since it’s good haskell practice.

I forgot a bit about the original PR, but the HasLink instance should look a lot like the HasClient instance.

A PR adding HasLink support for DeepQuery should also add it to QueryString (the subtle difference is that DeepQuery qs params should be added to the query string, while QueryString qs params should replace the query string (see the difference between the two HasClient impls).

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

3 participants