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

Support Custom JS Imports #224

Closed
FreeMasen opened this issue Jun 1, 2018 · 11 comments
Closed

Support Custom JS Imports #224

FreeMasen opened this issue Jun 1, 2018 · 11 comments

Comments

@FreeMasen
Copy link
Contributor

An issue was filed in the rand crate to add wasm-bindgen support.

rust-random/rand#478

This is currently stalled because the maintainers there don't want to use Math.random the only other option would be to include the native crypto functions. The problem with this is that in the browser you would use window.Crypto but in node it would be require('crypto').

Currently wasm-bindgen is using

const TextDecoder = typeof self === 'object' && self.TextDecoder
    ? self.TextDecoder
    : require('util').TextDecoder;

When it encounters a similar problem.

One solution for this would be to allow for arbitrary JS to be included in the wbg js output.

This would allow library developers to utilize the same tricks that wbg is using itself.

One example of how this might look would be

//./src/js/crypto.js
const _randomBytes = typeof self === 'object' && self.Crypto
    ? browserRng
    : nodeRng;

function browserRng(size) {
    let ret = new Uint8Array(size);
    return self.Crypto.getRandomValues(ret);
}

function nodeRng(size) {
    return require('crypto').randomBytes(size);
}
export function randomBytes(size) {
    return _randomBytes(size);
}
//./src/lib.rs
// ...extern, use blah blah
#[wasm-bindgen(import = "/js/crypto.js")]
extern {
  #[wasm-bindgen]
  fn randomBytes() -> Vec<u8>;
}
///...more work, blah blah

I am sure there is a lot more this that what I was able to outline above, I am curious what the community things about how we might go about doing something like this?

@fitzgen
Copy link
Member

fitzgen commented Jun 1, 2018

One example of how this might look would be

This is what I would expect.

@FreeMasen
Copy link
Contributor Author

I have been looking through the ast.rs file in the backend crate and it looks like there is already the #[wasmbindgen(module = "/js/import.js")] flavor of the macro. I am curious if this should be an additional argument like #[wasmbindgen(module = "/js/import.js", buldle)] or if it should be it's own argument?

@fitzgen
Copy link
Member

fitzgen commented Jun 5, 2018

I am curious if this should be an additional argument like #[wasmbindgen(module = "/js/import.js", buldle)] or if it should be it's own argument?

I'm not sure I follow -- can you explain some more?

@FreeMasen
Copy link
Contributor Author

FreeMasen commented Jun 5, 2018

We could add a new case to the BindgenAttr enum to enable this or we could update the BindgenAttr::Module(String) to BindgenAttr::Module(String, bool). Which would update the macro usage to look something like this:

The behavior of this would not change
#[wasm-bindgen(module = "./crypto.js")] == BindgenAttr::Module("./crypto.js", false)

This would attempt to find the file provided and append the contents of that file to the bindgen'd js file
#[wasm-bindgen(module = "./crypto.js", bundle)] == BindgenAttr::Module("/.crypto.js", true)

Hopefully that clears things up?

@fitzgen
Copy link
Member

fitzgen commented Jun 5, 2018

I would expect it to be the JS bundler's job to concatenate JS modules together, rather than wasm-bindgen's job to do that. Is there a reason that expectation is unrealistic?

@FreeMasen
Copy link
Contributor Author

Maybe I was unclear with my initial description of the issue.

Since we are targeting both node and the browser,

Developer's of Rust libraries, like the rand crate, might need to include arbitrary bits of javascript to become compatible with #[wasm-bindgen]. One way that could be done would be to allow them to add to these bits of javascript to the file generated by wasm-bindgen.

Maybe, this is the wrong direction?

@alexcrichton
Copy link
Contributor

Thanks for opening an issue for this, I'd meant to do so a long time ago and hadn't gotten around to it!

I think there's a lot of design space here and I think it's also gonna be pretty difficult to solve all the constraints (depending on how many we'd like to bring on). I definitely think that wasm-bindgen needs a story for "hey I wrote a small snippet of JS and I'd like to include it here".

I definitely agree with @fitzgen that ultimately it's gonna be the bundler's job to actually interpret a lot of the JS here and do final postprocessing for syntax and such. The main question to me is how to we actually get these files from the local filesystem and in-crate directives over to the bundler. For that I can think of:

  • One option is to not actually touch any files on the filesystem. I've seen various thoughts about this in the past saying that it's very important to never move the files on the filesystem to avoid disturbing relative paths. This would probably mean that we then record the absolute path to the referenced file in the wasm custom section. Later on when we generate a JS file to feed to the bundler we'd encode those absolute paths which would then all get concatenated as usual.

    A downside of this approach though is that publishing to NPM may be quite difficult. The generated JS would reference absolute paths that don't work across all systems, so it's not clear how we'd compile a wasm file with some crate using a snippet of JS and then push that up to the NPM registry. Maybe a bundler is used to compile code to publish to NPM though?

  • Another option would be to actually move files around on the filesystem (directly contradicting the advice received previously above). Doing something like this when you include a custom JS file we could, for example, encode the entire file into the custom section. That way you'd have a "fat wasm binary" which actually contains a bunch of JS files internally. When wasm-bindgen executes it'd then extract all these JS files and place them in appropriate locations.

    This would clearly solve the NPM use case without need for a bundler, but it would not solve the use case of these JS snippets depending on more than one file. We could possible extend the syntax here and there to accomodate dependencies but it wouldn't be great.

I'm not entirely sold on any particular strategy yet, but I'd definitely love to explore this!

cc @lukewagner, we've discussed this in the past as well

@lukewagner
Copy link
Contributor

I definitely like the idea of being able to include arbitrary bits of JS (written as ESMs) in the crate and the original comment is a great use case. My default assumption is that said glue wouldn't be injected into the wasm-bindgen-generated glue ESM. You'd just have either:
a.wasm --imports--> wbg-glue.js --imports--> rand.js
or, if no wasm-bindgen glue was actually necessary:
a.wasm --imports--> rand.js
and it would be the bundler's job to optimize this by smashing wbg-glue.js and rand.js together as appropriate.

@Pauan
Copy link
Contributor

Pauan commented Jul 2, 2018

I had written some posts covering this problem in a lot of detail (and offering a good solution):

rustwasm/team#92 (comment)
rustwasm/team#36 (comment)
rustwasm/team#36 (comment)

I hope they're helpful.

@Pauan
Copy link
Contributor

Pauan commented Jul 15, 2018

By the way, the above posts are a little outdated (you should mentally replace rustc with wasm-bindgen, and #[wasm_import_module = "./foo.js"] with #[wasm_bindgen(module = "./foo.js")]), but the general idea should still be correct.

@alexcrichton
Copy link
Contributor

Implemented in #1295!

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

No branches or pull requests

5 participants