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

Rust implementation #131

Closed
wants to merge 9 commits into from
Closed

Rust implementation #131

wants to merge 9 commits into from

Conversation

garritfra
Copy link
Member

@garritfra garritfra commented Jan 18, 2020

This might come out of the blue: I've tried rewriting a tiny function in rust to see what its like, and if it works. And sure enough, it works like a charm! I was able to implement the function isSupportedNodeVersion in rust. I chose this function, because it does not have a vital impact, yet its easy to implement. Please let me know what you think of this.

TODO

  • Fix Lint
  • Figure out way to publish binaries
  • Remove old utils module
  • Find way to recompile after changes
  • Add compile instructions to README

@garritfra garritfra added Idea This is an idea or a suggestion Discussion Discussions going on labels Jan 18, 2020
@garritfra garritfra changed the title Rust implementation [WIP] Rust implementation Jan 18, 2020
@garritfra
Copy link
Member Author

Mental note: Linting fails because nested node modules are not ignored by ESLint

@@ -1,7 +1,9 @@
#!/usr/bin/env node

const chalk = require("chalk");
const { isSupportedNodeVersion } = require("../utils/versionUtils");

// TODO: How to link lerna with unpublished package?
Copy link
Member Author

Choose a reason for hiding this comment

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

@andreyluiz any idea how to fix this?

Copy link
Contributor

Choose a reason for hiding this comment

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

You need to add the dependency for the clio-utils@0.1.0 package on the clio project package.json. Npm will automagically use lerna to know where to get the package.

@@ -0,0 +1,17 @@
[package]
name = "utils"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
name = "utils"
name = "clio-utils"

Copy link
Member Author

Choose a reason for hiding this comment

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

Didn't work either🧐

@garritfra
Copy link
Member Author

Evaluation concluded that Rust isn't really suited for node modules. We would either have to ship compiled binaries for every platform, host these binaries somewhere and download them on install time or compile it during installation of clio. None of these are valid options for us. We will however consider writing native modules in C/C++ in the future, given that virtually anyone has a corresponding compiler on their system.

@garritfra garritfra closed this Jan 18, 2020
@pouya-eghbali
Copy link
Member

pouya-eghbali commented Jan 18, 2020

@garritfra we can still do rust, and compile to universal wasm binaries, and ship the wasm files instead

@garritfra
Copy link
Member Author

@pouya-eghbali that's probably faster than js, but it doesn't give us native performance

@garritfra garritfra changed the title [WIP] Rust implementation Rust implementation Jan 19, 2020
@garritfra garritfra deleted the rust-integration branch January 25, 2020 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion Discussions going on Idea This is an idea or a suggestion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants