with the intention of binding null = nullImpl in the purescript source. This fails with a purescript error: “Unable to parse foreign module”.
Does anybody know how get this working? I’ve got everything else in foreign-generic converted. It mostly was using Proxy instead of the older proxy types.
// Here is the only example where you might need
// to modify FFI by hand.
//
// Sometimes, defining the function and then exporting it under
// a different name is needed to prevent issues with JavaScript
// keywords. For example, we might use the below FFI
// to export a function named `new`
// foreign import new :: Effect SomeObject
const newImpl = function () { return new SomeObject; }
export { newImpl as new };
I think that would make your example into this:
const nullImpl = null;
export { nullImpl as null };
const undefinedImpl = undefined;
export { undefinedImpl as undefined };
No, that didn’t work either. The error message mentions the token “export” in the error message, but I’ve tried every combination that I think is legal. From looking at the error detail, it seems the compiler might be parsing the .js file as purescript.
I tried to recreate this with a fresh project where I have a file Test.purs
module Test where
import Prelude
import Data.Maybe (Maybe(..))
import Foreign (F, Foreign, isNull, isUndefined)
-- | Read a value which may be null or undefined.
readNullOrUndefined :: forall a. (Foreign -> F a) -> Foreign -> F (Maybe a)
readNullOrUndefined _ value | isNull value || isUndefined value = pure Nothing
readNullOrUndefined f value = Just <$> f value
foreign import undefinedImpl :: Foreign
foreign import nullImpl :: Foreign
undefined :: Foreign
undefined = undefinedImpl
null :: Foreign
null = nullImpl
Ah and by the way - just to make sure - try to remove all of output (delete the folder) and recompile - maybe it’s some other old build artifact strangely interfering?
That fixed it. I never had purescript 0.14.X installed on this machine, so it doesn’t make sense.
I’m down to one compile problem in the test folder. I can’t figure it out. I’ve pushed the code to my github fork if someone would like to take a look at it. jsparkes/purescript-foreign-generic at v0.15 (github.com) It will be useful for anyone else wanting to port to 0.15.
Also, would it be useful to upgrade the tests into a test framework? Which one should I use? I mainly use spec myself.
The error is:
[{
“resource”: “/d:/src/purescript/purescript-foreign-generic/test/Types.purs”,
“owner”: “generated_diagnostic_collection_name#0”,
“code”: “NoInstanceFound”,
“severity”: 8,
“message”: " No type class instance was found for\n\n Foreign.Generic.Class.EncodeWithOptions a2\n\n The following instance partially overlaps the above constraint, which means the rest of its instance chain will not be considered:\n\n Foreign.Generic.Class.encodeWithOptionsRecord\n\n\nwhile solving type class constraint\n\n Foreign.Generic.Class.GenericEncodeArgs (Argument a2)\n\nwhile applying a function genericEncode\n of type Generic t0 t1 => GenericEncode t1 => { fieldTransform :: String → String\n , sumEncoding :: SumEncoding\n , unwrapSingleArguments :: Boolean\n , unwrapSingleConstructors :: Boolean\n }\n → t0 → Foreign\n to argument defaultOptions\nwhile inferring the type of genericEncode defaultOptions\nin value declaration encodeTree\n\nwhere a2 is a rigid type variable\n bound at (line 0, column 0 - line 0, column 0)\n t0 is an unknown type\n t1 is an unknown type\n",
“source”: “PureScript”,
“startLineNumber”: 102,
“startColumn”: 14,
“endLineNumber”: 102,
“endColumn”: 42
}]
I’m pretty sure you aren’t encountering a bug here; it looks instead to me like you’ve uncovered an ambiguity in foreign-generic that the 0.14 compiler let slide but the 0.15 compiler (correctly) takes issue with.
The instance chain for EncodeWithOptions needs to know whether the argument it’s being provided with is a Record. In 0.14, if you asked for an EncodeWithOptions a in a place where the compiler didn’t know if a was a Record, the Record-specific instance was skipped and the fully generic instance was used. Now, the compiler raises the error you see.
Ultimately, it seems that this prevents a generic instance for a Tree a from being created without knowing what a is. If it’s a Record, the assembled encoder is going to be different than if it isn’t, because the selected instance will be different.
One way to make the project compile would be to enhance the encodeTree instance with an EncodeWithOptions a constraint in addition to the Encode a constraint. This would essentially punt the problem up the stack, to the part of the tests that actually tries encoding a concrete Tree type.
A more aggressive fix would be to shuffle around the implementation of the record part of EncodeWithOptions. Since there’s already an instance for Encode (Record a), it’s not clear to me why that chain couldn’t be replaced with just the encodeWithOptionsOther instance, with the implementation of record encoding moved to the encodeRecord instance. I’m not 100% sure that won’t cause you other problems, though.
Indeed this is a simple fix where it compiles and the test succeed:
instance decodeTree :: (Decode a, DecodeWithOptions a) => Decode (Tree a) where
decode x = genericDecode defaultOptions x
instance encodeTree :: (Encode a, EncodeWithOptions a) => Encode (Tree a) where
encode x = genericEncode defaultOptions x
Thinning out the chain compiles after a few other fixes but ends up with a recursive call (stack-overflow).
Yes, that works, and the tests pass for me. Did the stack overflow come from the tests? Not that I’m at a stage where I could easily fix problems like that.
What shouild I do next, now that the port to 0.15 is complete? Do I just make a pull request for Freeman code bases?
I did not try to further fix this but just removing the the Record instance (the first one in the 2 part chain) ends up in a infinite loop when trying to encode/decode (you need change both to compile).
IMHO it’s probably a good idea to rethink the approach taken with this library but I think it would take me quite some time to really grasp the interconnected classes and resolution flow to even try such a fix.
So I think best first give a quick fix to 0.15 and then try to fix it or maybe open an issue.
Obviously @rhendric (and a lot other folks here) have a much deeper understanding on the issues here and could probably fix it quickly.
To be honest: I’m interested in fixing this myself (let’s say I want to pick up that gauntlet and restore my honor ) but right now I don’t have the time.
Concerning the PR - yes I think you should do this. I think there are people around that still maintain those even if the original author probably don’t any more (I think - not sure so sorry if I’m wrong)
Turns out @paf311 still accepted the last few PRs - so yes I think you work will find it’s way.
I don’t maintain this repo any more, and it looks like there is some discussion on the PR. It took me a while to get builds working again but adding the extra constraints seems to make the tests pass for me.