Javascript conundrum for ES6 module in foreign-generic

I’m trying to port purescript-foreign-generic to 0.15. (I want to use purescript-axios under 0.15, and it depends on foreign-generic)

Converting purescript-foreign-generic/NullOrUndefined.js at master · paf31/purescript-foreign-generic (github.com) to use ES6 modules has passed the limit of my javascript knowledge.

The original is

exports['null'] = null;

exports['undefined'] = undefined;

The simple conversion is

export const null = null;
export const undefined = undefined;

but it fails because javascript doesn’t allow a variable name null.

I’ve tried

export const nullImpl = null;
export const undefined = undefined;

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.

I think you are on the right track - can you change undefined to undefinedImpl also?

PS: maybe the parser has trouble with export const in this case you could try export let)

seems like the used parser-library has tests concerning export const for at least 3 years so I think this is not the issue here.

Does this section from the 0.15 Migration Guide help?

// 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.

[{
“resource”: “/d:/src/purescript/purescript-foreign-generic/src/Foreign/NullOrUndefined.purs”,
“owner”: “generated_diagnostic_collection_name#1”,
“code”: “ErrorParsingFFIModule”,
“severity”: 8,
“message”: " Unable to parse foreign module:\n\n d:\src\purescript\purescript-foreign-generic\src\Foreign\NullOrUndefined.js\n\n\n The module could not be parsed:\n\n\n ExportToken {tokenSpan = TokenPn 23 2 1, tokenLiteral = “export”, tokenComment = [WhiteSpace (TokenPn 22 1 23) “\n”]}\n",
“source”: “PureScript”,
“startLineNumber”: 1,
“startColumn”: 1,
“endLineNumber”: 15,
“endColumn”: 31
},{
“resource”: “/d:/src/purescript/purescript-foreign-generic/src/Foreign/NullOrUndefined.purs”,
“owner”: “generated_diagnostic_collection_name#1”,
“code”: “ErrorParsingFFIModule”,
“severity”: 8,
“message”: " Unable to parse foreign module:\n\n d:\src\purescript\purescript-foreign-generic\src\Foreign\NullOrUndefined.js\n\n\n The module could not be parsed:\n\n\n ExportToken {tokenSpan = TokenPn 23 2 1, tokenLiteral = “export”, tokenComment = [WhiteSpace (TokenPn 22 1 23) “\n”]}\n",
“source”: “PureScript”,
“startLineNumber”: 1,
“startColumn”: 1,
“endLineNumber”: 15,
“endColumn”: 31
}]

can you post the .js file?
I think this is some wrong character or something like this.

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

and Test.js

export const nullImpl = null

export const undefinedImpl = undefined

and this compiles fine for me - can you try this (obviously don’t include/name it Test)?

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?

I would expect this solution to work.

Is it possible that you are accidentally using version 0.14.x of the compiler? I wouldn’t expect to see the ErrorParsingFFIModule error on 0.15.

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
}]

Helllo,

as far as I can see is this a bug in the compiler/type-checker and it seems there is already an issue for this. ~~

I think you will probably have to wait till it’s resolved.

EDIT: Sorry my bad - it’s not that issue (or maybe none at all)

Hey @jsparkes, @CarstenKoenig,

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.

2 Likes

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).

For now I think the easy fix is good enough(?)

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 :smile: ) 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.

By the way: Thanks for helping out!

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.

Seems there was already an solution (guess we could have checked sorry) and we were a bit on a tangent.

Thanks