Implement transform classes

[dcodeIO]

This PR explores an alternative to #866. This is backwards-compatible to transform modules but now also supports classes that are populated with additional utility. On top of that this utilizes ts-node to run transforms written in TS without the need to build them.

[dcodeIO]

This now supports transforms written in TS without the need to build them. Haven't tested in detail, but the example works.

[willemneal]

The reason for building the typescript file was to bundle the transformer. This way you only need to provide one file. Also in situations like the browser it means a required ts-node/typescript dependency.

[MaxGraey]

For browser you should write on pure JS or transpile to JS from TS offline as usual. Current solution support ts-node optionally.

[MaxGraey]

typescript allow output to single bundle since 1.8. You don't need webpack for this.

[willemneal]

Ah cool! But still my point is that you if want to develop offline you need the proper types and that's been the headache for me.

[dcodeIO]

Last time I checked outFile had some restrictions that prevented using it with modules. What I'd usually do to bundle a transform is to declare assemblyscript as an external dependency that doesn't become bundled along the module. This way one can have bundles of assemblyscript and the transform that work together without duplicating any code.

Not sure about the types issue you are describing, but if you for example need SourceKind from the AssemblyScript package (as in the example), that'd have to be an import anyway which naturally also provides all the types.

[willemneal]

Okay thanks for the info. I was able to get everything to work with webpack, but tsc is still mad that there are two definitions for primitives, one in portable/index.d.ts and the other in assemblyscript.d.ts. Is there any situation when you would not want to types from portable since we aim to use the assemblyscript type definitions eventually?

[dcodeIO]

I think the portable definitions shouldn't be included there since transforms are TS, not portable AS. Does this come from your tsconfig? If so, try to make it a normal TS project.

[willemneal]

I did, but assemblyscript.d.ts uses definitions from portable. Also the idea in my mind is that these transformers would end up in assemblyscript at some point too so that is why I used the portable definitions.

[dcodeIO]

So I guess ideally we'd inline these in the .d.ts, essentially replacing their occurrences with number etc. directly (or make them otherwise private to the .d.ts (does removing declare there do this?)) - or are there other conflicting definitions?

[willemneal]

If we can always expect that there will be global definitions for the primitives, why can't we just remove it? Wouldn't you always want to have portable or assembly types in your types path?

[dcodeIO]

Wouldn't you always want to have portable or assembly types in your types path?

Not necessarily. There are the following cases to consider:

1. Transform in JS (no definitions)
2. Transform in TS (assemblyscript.d.ts)
3. Transform in portable AS (assemblyscript.d.ts and portable.d.ts)
4. Transform in AS (assemblyscript.d.ts and assembly.d.ts)

with 3. and 4. being problematic apparently. However, 2. is a valid case as well in which case the .d.ts currently needs the aliases because the compiler itself is portable AS. Hence my suggestion to do a pass over assemblyscript.d.ts replacing all the basic types with their actual alias.

[dcodeIO]

Last commit does that now, that is replace all occurrences of i32, f64 and whatnot with the respective JS types so we don't need the global type declarations anymore. If you'd like to try it out, make sure to run node scripts/build-dts to update.

[willemneal]

Cool that works on my end. Thanks!

[dcodeIO]

Great, let me know if there's something else necessary here, or if it's good to merge.

[willemneal]

My one last suggestion is to add a postCompile function that passes the binaryen module. This way a custom section could be added or more specific validation can be done, for example testing for floating point numbers.

[dcodeIO]

Alright, added an afterCompile hook that is being called with the Module instance. The examples show how to wrap it in a binaryen.js module. Process continues after modification by verifying the module before emitting it according to compiler options.

It's important to use the exact Binaryen dependency used by asc for this because wrapping like this requires that all the data is in Binaryen's memory already.

[willemneal]

Thanks! Why must the module be rewrapped? I thought that the one passed was already wrapped by the compiler's module class.

[dcodeIO]

It is not ultimately necessary to do that, but I'd imagine that the binaryen.js API is more friendly to consume than the internal C-API wrapper. If the internal API is good enough, one can ofc use that instead. Just note that extending the internal Module class is limited to portable AS and not every feature that might be possible is necessary to include in the compiler. For instance, if a consumer of the internal API finds that they need feature X, but we don't want that to bloat the compiler code, the respective PR might become rejected.

[willemneal]

Ah cool well then it LGTM!

[willemneal]

Oh one last thing, can there be a try catch surrounding the each call to the transform class?

[MaxGraey]

I guess better if try/catch will be inside afterParse / afterCompile and determine by user. This also make possible catch errors and decide how this handle.

[willemneal]

Yeah but since the current style is to call the callback with the error, it would be good if the transforms can use the callback to check if their error occurred.

[MaxGraey]

May be better have a possibility interrupt next process and gracefully shutdown compiler like:

class MyTransform extends Transform {
  afterParse(parser: Parser): boolean {
     try {
        ....
        return true // ok
     } catch (e) {
        // handle error
       return false // interrupt and skip next transforms
    }
  }

...
function applyTransform(name, ...args) {
    var result = transforms.every(transform => {
      if (typeof transform[name] === "function") {
         return transform[name](...args));
      }
      return true;
    });
    if (!result) throw new Error(...); // or pass this to return or better shutdown compiler
  }
...

[dcodeIO]

What this now does it to abort compilation on the first transform throwing, and propagates the error to the callback function. In case of the default callback, the exit code will be 1. That sufficient?

[dcodeIO]

Merging so we can generate an updated assemblyscript.d.ts along with the recent Binaryen update. If there's anything else, you know what to do :)

[willemneal]

Thanks!