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

Replace and deprecate HList, FHList, distributeFHListOverDynPure and collectDynPure. #106

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from

Conversation

adamConnerSax
Copy link

This replaces/deprecates HList and FHList with the NP type from generics-sop. It has equivalent functionality and much greater support from the generics-sop library. It replaces/deprecates dsitributeFHListOverDynPure and collectDynPure with equivalent functionality built from NP and replaces the use of distributeFHListOverDynPure in qDynPure (in Reflex.Dynamic.TH) with the NP version.
This version of collectDynPure is more powerful, or at least easier to deploy, than the previous one since it requires only that the types be instances of generic-sop's Generic type (easily derived from the GHC Generic type), rather than needing to write HList and FHList instances.
It adds tests of this functionality: a standalone Pure-only test comparing mkDynPure quasiquoting with zipDynWith and a pure vs spider comparison of mkDynPure in the semantics test suite. More testing might be useful.
It adds some supporting functions for converting generics-sop NP to dependent maps and back. This should properly be in a separate library but until that is done, it is placed here, in Generics.SOP.DMapUtilties.
This adds one library dependency, namely on generics-sop (>=0.2.4.0).
Older types and functions have been retained but deprecated.

-- |A Tag type for making a 'DM.DMap' keyed by a type-level list
data TypeListTag (xs :: [k]) (x :: k) where -- x is in xs
Here :: TypeListTag (x ': xs) x -- x begins xs
There :: TypeListTag xs x -> TypeListTag (y ': xs) x -- given that x is in xs, x is also in (y ': xs)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here and There are very nearly a naming conflict with here and there in Data.These. Are these names chosen to be consistent with something else, or would it be possible to change them to avoid this near-conflict?

maybeDyn :: (Reflex t, MonadHold t m, MonadFix m) => Dynamic t (Maybe a) -> m (Dynamic t (Maybe (Dynamic t a)))
maybeDyn = factorDynGeneric

eitherDyn :: (Reflex t, MonadHold t m, MonadFix m) => Dynamic t (Either a b) -> m (Dynamic t (Either (Dynamic t a) (Dynamic t b)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really cool!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I still think there may be a bug but though I first found it a while ago, I haven't had a chance to go back and update the reflex version (maybe I'll try that now) or make a simpler case (I found it amidst all the profunctor stuff). But we should definitely make sure there are a few tests.

@ryantrinkle
Copy link
Member

This looks really awesome! I've updated it on the generics-sop branch to the latest reflex develop. However, it looks like generics-sop uses a lot of template-haskell, which means we can't support this on mobile :( Currently, everything in reflex supports mobile fully, so we'll need to think about how to approach this. I'd really like to get this in!

@adamConnerSax
Copy link
Author

I'd be happy to help get it in. I've got no idea how to work around the TH issue for mobile. It might be the case that it only interferes with factorDyn and not all the HList replacement stuff since it only comes into generics-sop for deriving generic instances. So it doesn't get used for the NP and NS types and so the HList replacement stuff might all be okay. But I have to look again.

@ryantrinkle
Copy link
Member

@adamConnerSax I just pushed a merged version of this here: https://github.com/reflex-frp/reflex/tree/factorDynGeneric-update

However, until well-typed/generics-sop#61 is fixed, it sounds like we probably can't use generics-sop.

@ryantrinkle ryantrinkle added this to the Later milestone Jan 4, 2018
@hSloan hSloan added wontfix clean up Removes stale code, improves read-ability, or small refactors labels Jan 23, 2019
@ali-abrar
Copy link
Member

It looks like generics-sop was split up so that there's a no-TH sop-core package. That package doesn't appear to have everything we'd need to get this branch working without TH, though.

@ali-abrar
Copy link
Member

But, I've been reminded that we don't need to avoid TH anymore anyhow.

@ryantrinkle ryantrinkle removed clean up Removes stale code, improves read-ability, or small refactors wontfix labels Mar 17, 2019
@ali-abrar ali-abrar changed the base branch from jsaddle to develop March 18, 2019 03:29
@ali-abrar
Copy link
Member

I'm working on updating this so that we can merge it.

@Ericson2314
Copy link
Member

We don't need non-TH anymore I don't think

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants