-
Notifications
You must be signed in to change notification settings - Fork 2
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
Clear Include before updateRecord
#3
base: main
Are you sure you want to change the base?
Conversation
Web/Controller/LandingPages.hs
Outdated
@@ -79,6 +72,11 @@ instance Controller LandingPagesController where | |||
|
|||
setSuccessMessage "LandingPage updated" | |||
redirectTo EditLandingPageAction { .. } | |||
where | |||
extractLandingPageFromInclude :: Include' ["paragraphCtas", "paragraphQuotes"] LandingPage -> LandingPage | |||
extractLandingPageFromInclude landingPage = landingPage |
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.
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.
can you try to split this into two sub functions: one turning Include' ["paragraphCtas", "paragraphQuotes"] LandingPage -> Include' ["paragraphCtas"] LandingPage
and then one turning Include' ["paragraphCtas"] LandingPage -> LandingPage
?
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.
That actually works, which isn't intuitive. a9f132a
Why is that (and how did you figure it might work 😄 )?
Is it somehow related to the fact that the compiler thinks that Include "paragraphQuotes" LandingPage
is not equal to Include' ["paragraphQuotes"] LandingPage
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 expected this by intuition 😄 In very early IHP versions updateField
was actually set
and it was later changed (because it was causing errors like these). The core problem with updateField
is, that it's type definition is very open. It's defined as updateField :: value' -> model -> model'
. Here model
and model'
are two independent type variables. This means a call to updateField
can actually change the return type of the record (e.g. turning LaindingPage
to Include "paragraphQuotes" LandingPage
). The problem with your first version was that GHC could not figure out the model'
variable because multiple updateField
were chained (the model
type argument is easy for GHC to figure out, it's just that the model'
is independent of that).
For comparison set
is defined as set :: value -> model -> model
. In that case when GHC can figure out model
it will never error. So set
is typically easy to figure out and unlikely to error. One the other side the problem with set
is that it's less flexible and cannot change the output type (e.g. with model = LandingPage
it will be set :: value -> LandingPage -> LandingPage
, so there's no way to express e.g. the Include
stuff). That's why updateField
exists.
TL;DR: updateField
has a more open type signature than set
and sometimes GHC needs a bit of help
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.
👌 Thanks for the explanation.
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.
@mpscholten I've updated Stackoverflow, so this info is not lost :)
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.
Thanks 👍
No description provided.