Transformer Class #866

willemneal · 2019-09-24T15:50:40Z

Also add error handling for transformers

dcodeIO · 2019-09-24T16:57:10Z

Wdyt of making each transform function a method on a Transform class, and the class provides all the additional utility instead of providing arguments? For example

class MyTransform extends Transform {
  afterParse() {}
}

which then has this.parser, this.writeFile, this.baseDir and so on? So asc would instantiate each transform, populate the utility and then call the hooks.

willemneal · 2019-09-24T22:55:22Z

Sounds good to me. Was trying to be minimal, but I'll switch to that.

Also add error handling for transformers

willemneal · 2019-10-10T02:29:39Z

Previously I had tried to depend directly on the webpacked module, but then it becomes packaged with the transformer. So I tried importing the source files directly. Unfortunately this also means that every file gets packaged with it too. However, since the classes are already packaged or present with the compiler, then you only need the types document.

Thus this is also the start of generating types for the AST that are separate from the compiler itself.

One thing that I didn't do yet, but is something to consider is what the typescript compiler does, which is to "brand" interfaces. Which makes

From their src/compiler/types.ts:

 // Note: 'brands' in our syntax nodes serve to give us a small amount of nominal typing.
// Consider 'Expression'.  Without the brand, 'Expression' is actually no different
// (structurally) than 'Node'.  Because of this you can pass any Node to a function that
// takes an Expression without any error.  By using the 'brands' we ensure that the type
// checker actually thinks you have something of the right type.  Note: the brands are
// never actually given values.  At runtime they have zero cost.
export interface Expression extends Node {
  _expressionBrand: any;
}
//....
export interface UnaryExpression extends Expression {
  _unaryExpressionBrand: any;
}

export interface UpdateExpression extends UnaryExpression {
  _updateExpressionBrand: any;
}

// see: https://tc39.github.io/ecma262/#prod-UpdateExpression
// see: https://tc39.github.io/ecma262/#prod-UnaryExpression
export type PrefixUnaryOperator
  = SyntaxKind.PlusPlusToken
  | SyntaxKind.MinusMinusToken
  | SyntaxKind.PlusToken
  | SyntaxKind.MinusToken
  | SyntaxKind.TildeToken
  | SyntaxKind.ExclamationToken;

export interface PrefixUnaryExpression extends UpdateExpression {
  kind: SyntaxKind.PrefixUnaryExpression;
  operator: PrefixUnaryOperator;
  operand: UnaryExpression;
}

This is useful because you have type assertions in typescript rather than type casting. So when I say

<PrefixUnaryExpression>expr it will assume from that point on that expr would be

type PrefixUnaryExpression = { 
  _expressionBrand: any;
  _unaryExpressionBrand: any;
  _updateExpressionBrand: any;
  kind: SyntaxKind.PrefixUnaryExpression;  
  operator: PrefixUnaryOperator;  
  operand: UnaryExpression;
}

I noticed this when I attempted to give the type of each Node, its NodeKind, but found there were instances where the node's parent had the NodeKind and the child couldn't overwrite the type.

dcodeIO · 2019-10-10T05:30:19Z

I feel like there's a tendency in your PRs to overstep the mark of what's necessary by a bit and I'd prefer if it remained simple. For instance, I imagine that having to keep another set of typings in sync with the compiler whenever something about the AST changes will result in all sorts of problems and unnecessary work. Ideally, there'd be typings for the transform one can optionally add by means of /// <reference ... >ing, which in turn can pull in the respective types for the AST nodes from assemblyscript.d.ts. This way around no dist files must be rebuilt as well.

willemneal · 2019-10-10T13:26:21Z

Lol that's why I started simple, but then with your suggestion I went for more because of my previous hardship with working with the types external from the compiler. As for your fear about changes with the AST, I think it's best to have a set of definitions that the compiler will conforms to and any external library will expect.

This is also the result of me trying to understand the typescript's typings files and webpack. I attempted to have everything to be global definitions, but enums have to be imported and type files can either be completely ambient or export definitions.

When I tried the last of what you mention I end up with two issues: one conflicting definitons of primitives in portable.d.ts and asssemblyscript.d.ts; and I still have to import from the bundle.

/// <reference types="../../../dist/assemblyscript" />
export * from "assemblyscript";

compiles to

"use strict";
/// <reference types="../../../dist/assemblyscript" />
function __export(m) {
    for (var p in m) if (!exports.hasOwnProperty(p)) exports[p] = m[p];
}
Object.defineProperty(exports, "__esModule", { value: true });
__export(require("assemblyscript"));

So although I just want the types, I still have to import the build js code. Which webpack really doesn't like.

I'm new at this stuff and could use any help.

MaxGraey · 2019-10-10T13:33:41Z

std/portable/index.js

-    value: function sincos(x) {
-      this.sincos_sin = Math.sin(x);
-      this.sincos_cos = Math.cos(x);
+try {


Why you wrap this to try/catch?

Ah because since my code depends on portable too, it causes it to be redefined. I suppose I could change this to check if already define.

MaxGraey · 2019-10-10T13:40:16Z

I wonder why we need webpack and ts-loader? Why this PR touches portable.js? Why so many changes for pretty trivial original goal (just improve Transformer class)?

willemneal · 2019-10-10T13:44:46Z

It's all because of the pain I had trying to compile a transformer from typescript. Also I didn't add webpack and ts-loader as those come from the root directory.

Again if you have a better way to compile this without what I've done, I'd much appreciate it. I tried to touch the current code as little as possible, but I wanted to try to make a tool that removed all the headaches involved with getting the types set up.

MaxGraey · 2019-10-10T14:32:57Z

lib/transformer/README.md

+/**
+ * Example of using a transformer to print the AST for each entry file
+ */
+export default class Printer extends Transformer {


According to latest best of practice for JS/TS we should avoid default exports

MaxGraey · 2019-10-10T14:35:02Z

cli/asc.js

+          transform[name](...args);
+        } else if (transform.default) {
+          // Export default class which needs to be constructor
+          if (!isConstructor(transform.default)){


That's works only for default exports which could cause to problems

MaxGraey · 2019-10-10T14:47:23Z

cli/asc.js

@@ -939,3 +965,7 @@ exports.tscOptions = {
  types: [],
  allowJs: false
 };
+
+function isConstructor(func) {
+  return (func && typeof func === "function" && func.prototype && func.prototype.constructor) === func;


should be

function isConstructor(fn) { return typeof fn === "function" && !!fn.prototype && fn.prototype.constructor === fn; }

Just check:

assert.equal(isConstructor(undefined), false); assert.equal(isConstructor(null), false);

Currently all this true

dcodeIO · 2019-10-10T14:59:37Z

Made a little proof of concept here: cdb4dd6

MaxGraey · 2019-10-10T15:00:30Z

It's all because of the pain I had trying to compile a transformer from typescript. Also I didn't add webpack and ts-loader as those come from the root directory.

In webpack you use "umd" and "source-map". In tsconfig "commonjs" and "inlineSourceMap". Did you try use module: "umd" in tsconfig? Or what kind of problem you have with tsc?

willemneal · 2019-10-10T16:26:35Z

I tried all sorts of combinations for tsc (I borrowed the webpack from the root one). Currently if I change module to "umd" in the config it doesn't understand the global definitions for portable.d.ts.

MaxGraey · 2019-10-10T16:31:09Z

I just wondering why you need portable.d.ts and portable.js at all for examples.

MaxGraey · 2019-10-10T16:46:49Z

lib/transformer/tests/package.json

@@ -0,0 +1,10 @@
+{
+  "name": "near-bindgen",


"near-bindgen"?

Oops! Yeah it was a test originally used there.

willemneal · 2019-10-10T17:35:52Z

I thought the goal would be to eventually use assemblyscript itself. If we created a simple interface is assemblyscript, we could compile and load it. For example, we could create decorators written in assemblyscript that can use builtins.

MaxGraey · 2019-10-19T11:48:02Z

@willemneal I guess we could close this since all proposed features already in master?

bowenwang1996 mentioned this pull request Sep 24, 2019

Replace fork with transforms nearprotocol/assemblyscript#74

Open

willemneal and others added 3 commits October 8, 2019 09:10

Add write file and baseDir to transform

261b8bf

Also add error handling for transformers

Merge branch '0.7' into dev_transform

8729f06

Added Transformer class

a5441c1

willemneal force-pushed the dev_transform branch from a4563b1 to a5441c1 Compare October 8, 2019 14:19

Willem Wyndham added 4 commits October 9, 2019 15:25

Added Webpack and made AST interfaces instead of classes

a564bda

Must pass webpack if another package wants to use the base file

0951215

Fix typo in files list

3ab14f9

Remove last dependency on asc's source

c068111

willemneal changed the title ~~Add write file and baseDir to transform~~ Transformer Class Oct 9, 2019

Add to CI tests

77dbb6c

willemneal mentioned this pull request Oct 10, 2019

Parser identifies ~lib/rt/index-full as SourceKind.USER_ENTRY #889

Closed

MaxGraey reviewed Oct 10, 2019

View reviewed changes

dcodeIO mentioned this pull request Oct 10, 2019

Implement transform classes #892

Merged

willemneal closed this Oct 19, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Transformer Class #866

Transformer Class #866

willemneal commented Sep 24, 2019

dcodeIO commented Sep 24, 2019

willemneal commented Sep 24, 2019

willemneal commented Oct 10, 2019

dcodeIO commented Oct 10, 2019

willemneal commented Oct 10, 2019

MaxGraey Oct 10, 2019

willemneal Oct 10, 2019

MaxGraey commented Oct 10, 2019

willemneal commented Oct 10, 2019

MaxGraey Oct 10, 2019

MaxGraey Oct 10, 2019

MaxGraey Oct 10, 2019 •

edited

Loading

dcodeIO commented Oct 10, 2019

MaxGraey commented Oct 10, 2019

willemneal commented Oct 10, 2019

MaxGraey commented Oct 10, 2019

MaxGraey Oct 10, 2019

willemneal Oct 10, 2019

willemneal commented Oct 10, 2019

MaxGraey commented Oct 19, 2019

Transformer Class #866

Transformer Class #866

Conversation

willemneal commented Sep 24, 2019

dcodeIO commented Sep 24, 2019

willemneal commented Sep 24, 2019

willemneal commented Oct 10, 2019

dcodeIO commented Oct 10, 2019

willemneal commented Oct 10, 2019

MaxGraey Oct 10, 2019

Choose a reason for hiding this comment

willemneal Oct 10, 2019

Choose a reason for hiding this comment

MaxGraey commented Oct 10, 2019

willemneal commented Oct 10, 2019

MaxGraey Oct 10, 2019

Choose a reason for hiding this comment

MaxGraey Oct 10, 2019

Choose a reason for hiding this comment

MaxGraey Oct 10, 2019 • edited Loading

Choose a reason for hiding this comment

dcodeIO commented Oct 10, 2019

MaxGraey commented Oct 10, 2019

willemneal commented Oct 10, 2019

MaxGraey commented Oct 10, 2019

MaxGraey Oct 10, 2019

Choose a reason for hiding this comment

willemneal Oct 10, 2019

Choose a reason for hiding this comment

willemneal commented Oct 10, 2019

MaxGraey commented Oct 19, 2019

MaxGraey Oct 10, 2019 •

edited

Loading