toActionInput throws input holes, fixes #194

[seanhess]

See #194

Instead of using undefined to serialize a partially applied constructor, we throw InputHole - we catch it during serialization and turn it into a Hole.

This inspired a refactor to remove Web.Hyperbole.Data.Param. Everything now uses ToJSON as a generic serialization format, then encodes in its own way. This changes QueryData, Session, and Encoded (ViewId, ViewAction)

[seanhess]

What about forcing the user to be more explicit? It might help users avoid doing crazy things with (input -> Action view)

In the latest push, onInput (Text -> Action view) works out of the box. Other types must implement UserInput. It doesn't assume it knows how you want to parse numbers from an <input>, for example.

-- From Web.Hyperbole.Data.Argument
class UserInput a where
  parseInput :: Text -> Either String a
  default parseInput :: (FromJSON a) => Text -> Either String a
  parseInput = decodeArgument


instance UserInput Text where
  parseInput = pure


instance {-# OVERLAPPABLE #-} (UserInput a) => UserInput (Maybe a) where
  parseInput "" = pure Nothing
  parseInput t = Just <$> parseInput @a t


instance {-# OVERLAPS #-} UserInput (Maybe Text) where
  parseInput = pure . Just

-- Examples: 


-- If you want another type as an input, you must implement UserInput. We don't automatically parse integers from search fields
newtype NumInput = NumInput Int
  deriving (Generic, Show, Eq)
  deriving anyclass (UserInput, FromJSON, ToJSON)

data Custom = Custom 
  deriving (Generic, ToJSON, FromJSON, UserInput)

data NotAnInput = NotAnInput Int Text
  deriving (Generic, ToJSON, FromJSON)

data SomeAction
   = IntegerInput Int NumInput
   | OutOfOrder NumInput Int
   | TextInput Text
   | NullableInput (Mabye Text)
   | CustomInput Custom
   | BadInput Int
   | BadInput2 NotAnInput
  deriving (Generic, ViewAction)
 
 -- These work
toActionInput TextInput
toActionInput NullableInput
toActionInput (IntegerInput 4)
toActionInput (CustomInput)
toActionInput (\n -> OutOfOrder n 4)
 
 -- Throws a runtime error
 toActionInput (NullableInput . Just)
 toActionInput (Nested . NumberPair 0)
 
 -- Compile Errors
toActionInput BadInput
toActionInput (NotAnInput 3)
toActionInput (\n -> NotAnInput n "hello")
toActionInput (BadInput . (*2))
toActionInput (IntegerInput 4 . (*2))

What do you think? How could it be improved? I think this might make people think twice before doing crazy things with the partial constructor.

[seanhess]

@kschweppe do you have any thoughts on this solution? I could use a second set of eyes on this.

[kschweppe]

Sure! Sorry for the delay, I just didn't get to have a closer look over the weekend.

I generally agree that it should be more explicit, and was also thinking we need a typeclass for this.
I still need to play around a bit more, but this seems to work well and cover most cases.

Is there any drawback in providing a default implementation for numbers? I guess having number dropdowns is a common use case, for example I have dropdowns for year and month and needed to add NumInput to both.

And then I'm wondering if this can/should also work with search input, which actually was my initial use case.
So I wanted something like search (SearchUser . SearchInput False) (which now at least throws a runtime error). Do you think it makes sense to use UserInput for other events as well, like onInput, or only for onChange?