Remove instance PFromJSVal a => PFromJSVal (Maybe a)

[achirkin]

Please, remove instance PFromJSVal a => PFromJSVal (Maybe a) from GHCJS.Marshal.Pure

This is especially important for enum types. Let say, I want map a string property of JS object onto dedicated Haskell data type:

data X = XoneOption | XuselessThing | Xanother
instance PFromJSVal (Maybe X) where
    pFromJSVal js = if not (isTruthy js)
        then Nothing
        else case pFromJSVal js :: JSString of
            "XoneOption"    -> Just XoneOption
            "XuselessThing" -> Just XuselessThing
            "Xanother"      -> Just Xanother
            _               -> Nothing

But current implementation does not make a lot of sense, because if a is an instance of PFromJSVal, one should probably take into account that underlying JS object may be null.

[mgsloan]

IMHO, in the case of null, an exception should be thrown. See #24 .

It is a bit strange to have this instance of maybe around, but it's also pretty darn convenient. You might be interested in my definition for Maybe in this PR: https://github.com/ghcjs/ghcjs-base/pull/34/files#diff-e09b67a14d374b52d6408c4ebcb9b630R7 . It ensures that you don't attempt to marshal Maybe (Maybe a)

[achirkin]

Thanks, you PR looks interesting.
However, in general I cannot confirm that instance of Maybe a is convenient, because it was not (completely) in my case.

In my particular case I do not want to fiddle with exceptions due to performance considerations and their uselessness (again, in my case). The idea is that I have some performance-critical part where I need to work with js objects transferred through network, and I do not want to transform them into Haskell representation (a lot of arrays there). So I have decided to only access their properties in pure code, so I need a lot of pure JSVal -> Maybe a. Moreover, being null or undefined property in that case is normal situation, so exceptions do not fit there.

Again, sometimes I cannot write PFromJSRef a, because I know it may be null or have wrong structure. However, automatic PFromJSRef (Maybe a) instance allows to check it only for being falsy, but not for having wrong structure.

Lastly, IMHO the idea of PFromJSRef (Maybe a) instance together with additional constraints against Maybe (Maybe a) is a bit of overkill. Actually the code of PFromJSRef (Maybe a) instance is only one line - not that much to fight for it, so if someone really neads a particular instance of it, they can write it one more time (as I did twice in my code already).

[mgsloan]

Moreover, being null or undefined property in that case is normal situation, so exceptions do not fit there.

So, it's only the instance for your enum type would throw exceptions for null. This seems like a valid thing to do for me. The instance for Maybe means that conversion from null to Maybe X will not throw an exception.

I'm starting to lean towards not having an instance for Maybe, though. One issue with the changes to the Maybe instance in my PR is that it only ensures that you don't end up with Maybe (Maybe ...). However, if you have Maybe (N a) where N is some type that can marshal to / from null, it does not catch this circumstance.

For now, in your particular case, I'd recommend using a newtype wrapper around Maybe.

[achirkin]

The main reason I do not want to use exceptions is that I expect my js values to be null - for a program it is not an exceptional behavior.

I am anyway using my own forked ghcjs-base, because of TypedArray implementation (current one is not useable for me), so I have just removed this Maybe instance from that code.