Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Compile grainc to JS & create binaries with pkg #570

Merged
merged 8 commits into from
Apr 18, 2021
Merged

Conversation

phated
Copy link
Member

@phated phated commented Mar 18, 2021

All my current work to get GrainJS working.

I believe I have some other ideas to actually just compile grainc to JS, so I've marked this as draft.

@phated phated force-pushed the grain-js-updated branch 2 times, most recently from 1f417ab to bc5f43b Compare March 24, 2021 07:16
@phated
Copy link
Member Author

phated commented Apr 1, 2021

@ospencer @peblair I believe this is pretty much ready from the packaging/virtual file system perspective. We just need to figure out the String allocation bug that happens in the JS version of the compiler. I would be forever grateful if either of you could help me debug it.

I'll probably be splitting this PR into small stuff that is more related to cleanup of the monorepo, but needed for this stuff to wrap up!

@ospencer
Copy link
Member

ospencer commented Apr 1, 2021

Awesome work! 🎉

I'll take a look at the allocation stuff.

@phated phated force-pushed the grain-js-updated branch 3 times, most recently from 0a17f7c to 1533726 Compare April 5, 2021 02:35
@phated phated changed the title WIP: Grain compiler as JS feat: Compile grainc to JS & create binaries with pkg Apr 5, 2021
@phated phated marked this pull request as ready for review April 5, 2021 02:46
@phated
Copy link
Member Author

phated commented Apr 5, 2021




@phated phated requested a review from a team April 5, 2021 02:57
@hhugo
Copy link

hhugo commented Apr 6, 2021

Is there anything to contribute back to js_of_ocaml ? Better support for unix stubs with node ?

@phated
Copy link
Member Author

phated commented Apr 6, 2021

@hhugo yeah! If you think the unix stubs would be helpful to JSOO, I'm happy to contribute them back!

cli/bin/compile.js Outdated Show resolved Hide resolved
cli/bin/grain.js Outdated Show resolved Hide resolved
Comment on lines +5 to -15
exec(file, options, { stdio: "inherit" });
return file.replace(/\.gr$/, ".gr.wasm");
} catch (e) {
console.log(e.stdout.toString());
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ospencer using stdio: inherit here instead of explicitly console.logging

cli/bin/exec.js Show resolved Hide resolved
process.exit();
} catch (e) {
console.log(e);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ospencer this uses stdio: inherit already, so we don't need to log the err here.

@phated
Copy link
Member Author

phated commented Apr 17, 2021

@ospencer hmm, there seems to be an issue with the lsp mode that I don't quite understand

Copy link
Member

@ospencer ospencer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved with passing tests!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants