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

AssemblyScript transformer #5598

Draft
wants to merge 3 commits into
base: v2
Choose a base branch
from
Draft

Conversation

aminya
Copy link
Contributor

@aminya aminya commented Jan 6, 2021

↪️ Pull Request

WIP. Traying to port the rollup plugin
https://github.com/surma/rollup-plugin-assemblyscript

💻 Examples

🚨 Test instructions

✔️ PR Todo

  • Added/updated unit tests for this change
  • Filled out test instructions (In case there aren't any unit tests)
  • Included links to related issues/PRs

@height
Copy link

height bot commented Jan 6, 2021

Link Height tasks by mentioning a task ID in the pull request title or description, commit messages, or comments.

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

@aminya aminya force-pushed the assemblyscript branch 2 times, most recently from ca9a304 to 2d99318 Compare January 9, 2021 02:10
// TODO port Rollup improts
code: `
import {compileStreaming} from "${SPECIAL_IMPORT}";
const wasmUrl = import.meta.ROLLUP_FILE_URL_${asset.id}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure how to port this

Copy link
Member

Choose a reason for hiding this comment

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

Conceptually, this should be the result of asset.addDependency() which gets replaced with the correct url.

Copy link
Member

@mischnic mischnic Jan 9, 2021

Choose a reason for hiding this comment

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

This might work:

let wasmCode = asc.compile(/*...*/);

asset.type = "js";
asset.setCode(`
import wasmURL from "my-wasm";
export const modulePromise = /*@__PURE__*/(() => compileStreaming(fetch(wasmUrl)))();
export const instancePromise = /*@__PURE__*/(() => modulePromise.then(module => WebAssembly.instantiate(module, {})))();
export default wasmUrl;
`)
asset.addURLDependency('my-wasm');

return [asset, {type: 'wasm', uniqueKey: 'my-wasm', code: wasmCode}];

Comment on lines 10 to 25
const MARKER = 'asc:';
const PREFIX_MATCHER = /^asc:(.+)$/;

// This special import contains the `compileStreaming` polyfill.
const SPECIAL_IMPORT = '__rollup-plugin-assemblyscript_compileStreaming';

const compileStreamingCode = `
export async function compileStreaming(respP) {
if('compileStreaming' in WebAssembly) {
return WebAssembly.compileStreaming(respP);
}
return respP
.then(resp => resp.arrayBuffer())
.then(buffer => WebAssembly.compile(buffer));
}
`;
Copy link
Contributor Author

@aminya aminya Jan 9, 2021

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

To share code between transformer outputs, you can put this into a separate file inside the transformer package like here:

// Add a dependency so Parcel includes the helpers
let moduleRoot = path.resolve(__dirname, '..', '..');
let helpersPath = path.resolve(__dirname, '..', 'esmodule-helpers.js');
let helperSpecifier = `@parcel/transformer-js/${normalizeSeparators(
path.relative(moduleRoot, helpersPath),
)}`;
asset?.addDependency({
moduleSpecifier: helperSpecifier,
resolveFrom: __filename,
env: {
includeNodeModules: {
'@parcel/transformer-js': true,
},
},
});
which includes
https://github.com/parcel-bundler/parcel/blob/e183e32c811f066310fff0610aa856ca73a2fcb6/packages/transformers/js/src/esmodule-helpers.js

wasmFilePath,
...(options.sourceMapURLPattern
? [
`--sourceMap=${renderNamePattern(options.sourceMapURLPattern, {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure how sourceMaps should be handled (renderNamePatter is a Rollup specific thing)


// wasm file
asset.type = 'wasm';
asset.setCode(await fsp.readFile(wasmFilePath));
Copy link
Member

Choose a reason for hiding this comment

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

Are there downsides to using this variant of the compiler instead? Would be cleaner than writing & reading from a tmpdir and would also work in the browser (from https://github.com/AssemblyScript/assemblyscript/tree/master/cli#api).

Bildschirmfoto 2021-01-09 um 16 20 52

@101arrowz
Copy link
Member

I'm a huge fan of AS, and this will definitely make using it more painless. However, I have a few concerns with this PR so far:

  • It's not necessary to use the CLI version over the string compiler, so I'd think it's better to use the string version
  • I think we should handle exports the same as with Rust - make it seamless without having modulePromise and instancePromise.
  • We could use a config, e.g. for transformations.
  • We might want to support stuff like process.env and process.browser and all the other runtime globals. This can be done with a Parcel internal transformation for AS.

Overall though, this looks very promising!

@mischnic
Copy link
Member

mischnic commented Jan 12, 2021

I think we should handle exports the same as with Rust - make it seamless without having modulePromise and instancePromise.

Is there a case where you need to pass an importObject with memory or imported functions to an AS wasm module? (e.g. can you import a JS function from AS?)

@101arrowz
Copy link
Member

Apparently so, AssemblyScript/assemblyscript#158...

@mischnic
Copy link
Member

Also found it here: https://www.assemblyscript.org/exports-and-imports.html#imports

So I guess we have to provide some why to pass them in.

This is also what I dislike about the proposal for importing Wasm with ES imports. You only get the exports but there is no way to provide imports. So there's no standardized way....

@101arrowz
Copy link
Member

The data to be imported from WASM could theoretically be provided via a config file, but I don't like the idea myself. At the same time, having different ways to import Rust WASM and AS WASM is just confusing and I'd rather sacrifice the ability to import from WASM than have to use both. The JS-WASM bridge is so slow that most WASM users probably don't need JS imports. A common strategy is to split logic between WASM and JS to avoid two passes across the bridge (the call and returning the value).

@aminya
Copy link
Contributor Author

aminya commented Mar 22, 2021

@aminya
Copy link
Contributor Author

aminya commented Apr 6, 2021

I tried to understand the Parcel API and mimic what the Rollup plugin does, but there are flaws in my implementation. Feel free to push to this branch.

@mischnic
Copy link
Member

mischnic commented Apr 11, 2021

I think Rollup and Parcel differ enough that it makes more sense to reimplement the features instead of a 1:1 port.

I've made a minimal version work (no automatic JS wrapper/runtime, the generated Wasm simply replaces the previous AS file) and added an example

We probably also need a config file for the assemblyscript compiler options


let code = await asset.getCode();
const {binary} = asc.compileString(code, {
// optimize: true,
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 user should be able to modify the options here

Copy link
Member

Choose a reason for hiding this comment

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

Yes, that's what I meant with

We probably also need a config file for the assemblyscript compiler options

@aminya
Copy link
Contributor Author

aminya commented Apr 11, 2021

As-bind is a killer feature and should be also implemented.

@aminya
Copy link
Contributor Author

aminya commented Apr 11, 2021

Automatic wasm instantiation would be nice, but as seen in the Rollup plugin, it requires supporting a notion like

import "asc:something.as"

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.

3 participants