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

[WIP] Support to rust/wasm #312

Merged
merged 8 commits into from
Dec 26, 2017
Merged

[WIP] Support to rust/wasm #312

merged 8 commits into from
Dec 26, 2017

Conversation

albizures
Copy link
Contributor

Hi, this PR is not complete yet, currently, I'm struggling with two things:

  • the first one is when there is a change in the entry rust file, the new wasm file is being generated but the file in the build folder doesn't change. I think it's something related to the cache.
  • and the second one is related to the secondary rust files when there is a change in these files parcel doesn't detect the change. I think I have to do something like in StylusAssets, use a custom evaluator, but I'm not sure.

Any ideas?

Thanks in advance

related: #34

@albizures albizures mentioned this pull request Dec 16, 2017
@devongovett
Copy link
Member

Awesome start, really excited about this! I'll give some more feedback in a bit! 🎉

@softprops
Copy link

Awesome to find some one is already working on this. So fast!

@brandon93s brandon93s changed the title Support to rust/wasm [WIP] Support to rust/wasm Dec 19, 2017
Copy link
Member

@devongovett devongovett left a comment

Choose a reason for hiding this comment

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

Great start! Left some suggestions inline.

const JSAsset = require('./JSAsset');
const localRequire = require('../utils/localRequire');

const rustTarget = `wasm32-unknown-emscripten`;
Copy link
Member

Choose a reason for hiding this comment

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

I think maybe we'll want to use wasm32-unknown-unknown, which doesn't require emscripten. Maybe checkout rustify which does something similar for browserify, and wasm-experiments to see how it all works.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right now I'm using wargo and for now, wargo just uses wasm32-unknown-emscripten

but sure, I'm going to take a look at rustify

this.dependencies.clear();
const {name, encoding, options} = this;
const release = process.env.NODE_ENV === 'production';
const cmd = `wargo build ${release ? ' --release' : ''}`;
Copy link
Member

Choose a reason for hiding this comment

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

did you mean cargo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, wargo is a library that setup your environment and makes easy to compile rust into wasm

Choose a reason for hiding this comment

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

Does this PR verify that wargo is installed and accessible or fail violently?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right now it's failing violently, 😄 but I was working on an api more friendly to use wargo directly instead of using wargo through child_process.
However, I was thinking about to stop using wargo since wargo is using wasm32-unknown-emscripten and I think wasm32-unknown-unknown is a better option...


const rustTarget = `wasm32-unknown-emscripten`;

class RustAsset extends JSAsset {
Copy link
Member

Choose a reason for hiding this comment

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

I think we should probably make a WASMAsset class to load precompiled .wasm files, and use it as a base class for this one. That way the runtime can be shared across multiple wasm languages.

async parse(code) {
this.invalidateBundle();
this.invalidate();
this.dependencies.clear();
Copy link
Member

Choose a reason for hiding this comment

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

why is this necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh my bad, this was me trying to fix the rebuild as mentioned in the description, I'll remove it, thanks

ENVIRONMENT: 'WEB'
};

if (options.fromHtml) {
Copy link
Member

Choose a reason for hiding this comment

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

Why is it different when loaded from HTML?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the difference is that with this approach you can use rust as a standalone executable, without javascript.

I got the idea from this article

@shawwn
Copy link
Contributor

shawwn commented Dec 21, 2017

Hey, I can set aside some time over the holidays to help finish the PR if you'd like. If you want a hand, ping me in parcel's Slack.

@albizures
Copy link
Contributor Author

Sure thing @shawwn thanks!

@albizures
Copy link
Contributor Author

Hey @devongovett, I made some changes

So, I got rid of wargo, and now I'm using wasm32-unknown-unknown directly also these changes fix my first point in the description 🎉

what is next?

  • tackle my second point in the description
  • validate the existence of rustc, wasm32-unknown-unknown target and create a useful alert
  • use wasm-gc, in production mode, if it exists

Do you have something else in mind?

also, I made a demo

cc: @shawwn

@devongovett
Copy link
Member

🎉 Awesome work @albizures! Your list sounds good.

I think you can use rustc to determine the list of imported files: use the --emit dep-info flag and it will generate a file with all the deps in it that we could parse. These can be sent to parcel as dependencies with the includedInParent: true flag, which will cause parcel to watch those files but not try to parse them as they've already been compiled by rust.

About validating the existence of rustc, I wonder if we could just install it for you if you don't have it installed. Especially since we depend on the nightly toolchain, a specific backend (wasm32-unknown-unknown), and wasm-gc, we could just install those for you if they aren't already. We do this for other compilers like typescript, but it's slightly easier since those are in npm.

Also, we should probably support using cargo build rather than rustc directly, if we detect a Cargo.toml file. This will allow some more complex builds to happen. Not sure if we can detect dependencies in that case though.

Let's also start thinking about what we want the API for importing wasm modules to be. I think there should be a small runtime built into Parcel to load and instantiate wasm modules. Basically we should take care of the WebAssembly.instantiate call and the import should resolve to instance.exports.

There are two cases to think about.

Synchronous import, wasm is either inlined into JS or somehow loaded prior to the JS running:

import {add} from './add.rs';

console.log(add(2, 3));

Asynchronous import, wasm is placed in a separate file and loaded on demand:

let {add} = await import('./add.rs');

console.log(add(2, 3));

Sorry for the long comment. All of this definitely doesn't need to happen in this PR, just getting the ideas flowing! 😄

@devongovett devongovett changed the base branch from master to wasm December 26, 2017 22:36
@devongovett
Copy link
Member

Going to merge this into the wasm branch on the main repo. I've added you as a collaborator @albizures so you should be able to work off that branch.

@devongovett devongovett merged commit 1f09abb into parcel-bundler:wasm Dec 26, 2017
@albizures
Copy link
Contributor Author

oh great, thanks 🎉

and I agree with you with all of your points.

I'll let you know any news

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.

5 participants