reasonml / reason-native

Testing, printing, coloring, and other tools to effectively write native Reason code.
https://reason-native.com/
MIT License
459 stars 43 forks source link

Missing primitives in rely with JSOO #207

Open Et7f3 opened 5 years ago

Et7f3 commented 5 years ago

Hy I wanted to update https://github.com/bryphe/reason-gl-matrix to rely. I was able to convert to rely natively but I can't run when building with JSOO.

I have some repro on my fork on branch Et7f3/test/switch_to_rely or commit instead https://github.com/Et7f3/reason-gl-matrix/commit/ec2fd9d2c5ea8ddc6f5172435cb49551e8db8aec

I quote some message from discord (https://ptb.discordapp.com/channels/235176658175262720/469417973820686336/641021659989540925).

Me:

For info it run fine on native with dune. I have tried only with a empty file that include Rely and I get some strange error maybe linked to str Image that show the error.

@bandersongit

You can also look at https://github.com/aweis/irmin-web as a currently working example (though it does appear to be pinned to an older version of Rely)

Me:

I have seen in irmin-web they have added --profile=release flag. When I add it I get some other error. Show missing primitives error.

@bandersongit

Looks like the issue with re is solveable by releasing a new version of Pastel

Pastel version should do it, got it working with a link to current master Fix with the new pastel

also had to add a stub for unix_isatty for pastel, I used


//Provides: unix_isatty

function unix_isatty (f) { return false; };


> I also changed your Testramework to

module MockSnapshotIO = { let readSnapshotNames = => []; let readFile: string => string = s => s; let removeFile: string => unit = => (); let writeFile: (string, string) => unit = (, ) => (); let existsFile: string => bool = => false; let mkdirp = => (); }; /* Snapshots will be virtual-modulified at some point, until then we need to inject

bandersongit commented 5 years ago

@Et7f3 A new version of Pastel has been released. I'll spend some time in the next couple days making it so that snapshots don't cause these errors if they aren't being used.

bandersongit commented 4 years ago

I filed https://github.com/ocsigen/js_of_ocaml/issues/913 to add unix_is_atty to JSOO along with my thoughts on implementation.

I've verified that your example now works if you provide a manual stub for unix_isatty, use the latest version of Pastel, and create a snapshot directory.

Et7f3 commented 4 years ago

I'm blind I don't see where you fix str issue 👀.

bandersongit commented 4 years ago

I'm blind I don't see where you fix str issue 👀.

@Et7f3 I removed references to str here https://github.com/facebookexperimental/reason-native/commit/d3203eaeb05b49837fa85b1daed4b31390b64e27, we just hadn't cut a Pastel release since then