-
Notifications
You must be signed in to change notification settings - Fork 11
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
Update functional dependencies so multiple operations can be supported on the same underlying record #2
Conversation
src/Heterogeneous/Folding.purs
Outdated
hfoldl :: f -> x -> a -> b | ||
|
||
class HFoldlWithIndex f x a b | a -> f x b where | ||
class HFoldlWithIndex f x a b | f a -> x b where |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curiously -- though I'm not sure exactly why -- this does not work:
class HFoldlWithIndex f x a b | f x a -> b where
test/Record.purs
Outdated
@@ -135,7 +136,7 @@ traverseRecord :: forall f k rin rout. | |||
f { | rout } | |||
traverseRecord k = | |||
map (flip Builder.build {}) <<< | |||
hfoldlWithIndex (TraverseProp k) (pure identity) | |||
hfoldlWithIndex (TraverseProp k :: TraverseProp f k) (pure identity) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MonoidMusician noticed that, after updating the functional dependencies, it's also necessary to annotate this now. This makes the library a little less user-friendly in that inference will nicely cover less cases.
test/Record.purs
Outdated
{ | rvals } -> | ||
{ | rin } -> | ||
{ | rout } | ||
testReplaceBoth vals = replaceRight vals <<< replaceLeft vals |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't been able to figure out why the RowToList
and MapRecordWithIndex
constraints are necessary. In addition, it's extremely brittle: MapRecordWithIndex rs (ReplaceLeft rvals) rout rout
does not work, for example.
Unfortunately, putting these as multiple constraints on a large function (as in thomashoneyman/purescript-halogen-formless@0811a64) completely messes up type checking when that function is used. Clearly my solution as provided here is not actually solving the problem, though it appears to on the tests that I've added in this module. ContextI’m mounting a component: Halogen.HTML.slot unit Formless.component ?abc (const Nothing) Now, I’m supposed to provide an argument where I’ve placed type Input f = { initialInputs :: f Record InputField, ... } I’ve gone ahead and made one using the explicit / verbose format: initialInputs :: ContactForm Record InputField
initialInputs = ContactForm { name: InputField "", text: InputField "" } the component has explicitly been told that Halogen.HTML.slot unit Formless.component { initialInputs, ... } (const Nothing) …I get an error:
Strangely, the compiler appears to believe I’ve provided something of type This happens even if I just put For context, here's the component, though it's a gnarly type signature: component
:: ∀ form is fs fxs
. Monad m
=> RL.RowToList fs fxs
=> HMap GetInputField { | fs } { | is }
=> HMap SetFormFieldTouched { | fs } { | fs }
=> MapRecordWithIndex fxs (ReplaceInput is) fs fs
=> HFoldl CountError Int { | fs } Int
=> HFoldl Touched Boolean { | fs } Boolean
=> Newtype (form Record InputField) { | is }
=> Newtype (form Record FormField) { | fs }
=> Component ParentQuery ChildQuery ChildSlot form m In the end, I reverted the changes and went back to a more primitive |
Thank you for looking into this. I've made a followup commit which will clean things up a little bit. 0ba771a To go into more detail. The crux of the issue is that with the old functional dependencies I actually think one way around it would be to use a fresh type variable for the input type, with In that commit, I've just fixed everything to the more general dependencies of In your |
I find that surprising -- wouldn't the type signature below assert to the compiler that the output row of both constraints is in fact the same row? Why would the compiler consider them to be actually If the compiler considers them to be HMapWithIndex (ReplaceLeft rvals) { | rin } { | rout } =>
HMapWithIndex (ReplaceRight rvals) { | rin } { | rout } => Aside from my questions, is there anything else that ought to happen in order to merge this? It seems that at least a README update could be included to note the new need to annotate some types. |
The issue is that you are feeding the result of one into the next. Your constraints say it goes from |
If you want to merge my branch into yours, and update the README, I'll merge it. |
Ah! That does make sense, in retrospect, especially looking at your adjustment in 0ba771a. My understanding of what these type variables mean is indistinct; I usually consider Do you have a better intuition for what makes |
You can use the same variable. This type checks. testReplaceBoth :: forall rvals r.
HMapWithIndex (ReplaceLeft rvals) { | r } { | r } =>
HMapWithIndex (ReplaceRight rvals) { | r } { | r } =>
{ | rvals } ->
{ | r } ->
{ | r }
testReplaceBoth vals =
replaceLeft vals >>> replaceRight vals But you didn't do that 😄, if you use distinct type variables, then they are distinct to the compiler. The only way to have them unify again is through some sort of equality. |
@natefaubion Updated! |
Thanks! |
Currently,
purescript-heterogeneous
allows you to create functions like these:But what if you want a function that uses multiple operations?
This ought to be valid, but instead causes a compiler error. A record can only match one instance!
Solution
This PR updates the functional dependencies for the
HFoldl
andHMap
classes (with help from @MonoidMusician) and provides test cases proving that the new functional dependencies support this use case.