search

grant / ts2gas

A function that transpiles TypeScript to Google Apps Script.
http://npmjs.com/ts2gas
MIT License
88 stars 11 forks source link

Inheritance class becomes TypeError in load order. #27

Closed jakenjarvis closed 5 years ago

jakenjarvis commented 5 years ago

Just because the inherited class has a different loading order(alphabetical), it becomes "TypeError: Cannot read property 'prototype' of undefined". Screen shot

I created a sample project to convey this.

Expected Behavior

case OK: https://github.com/jakenjarvis/clasp-ts-sample/tree/master/IssueSample/Issue01/ProjectNormal

Actual Behavior

case NG: https://github.com/jakenjarvis/clasp-ts-sample/tree/master/IssueSample/Issue01/ProjectAbnormal

TypeError: Cannot read property 'prototype' of undefined (line 14, file "AClass")

Steps to Reproduce the Problem

  1. Swap the class names of AClass and BClass.
  2. Oh wonder, it becomes TypeError.

Specifications

I posted the same issue here(Clasp#527). I was urged to instead Issue to ts2gas.

grant commented 5 years ago

Can you please put the code that is wrong in the GitHub issue?

Please provide a simple test case.

jakenjarvis commented 5 years ago

@grant thank you for confirmation. I'm not good at English, so it is difficult to explain. So I decided to show the source code. My explanation may not have been enough.

This sample presented by me is a simple test case. Please also check the README.

If you want to build a project from scratch, please create the following class configuration. Be sure to check the operation on GAS. An error occurs at run time.

Create a normal project

AClass.ts

export class AClass {
  // (omission)
}

BClass.ts

import { AClass } from './AClass';
export class BClass extends AClass {
  // (omission)
}

Create a abnormal project

Swap class names(file names). Specifically, it is as follows. The difference is only to reverse the parent-child relationship. As a result, this will reverse the load order. (It appears to be loaded in alphabetical order)

AClass.ts

import { BClass } from './BClass';
export class AClass extends BClass {
  // (omission)
}

BClass.ts

export class BClass {
  // (omission)
}

Perhaps we need a function to automatically determine load order by parsing import. I currently avoid the problem by devising the class name, but I strongly hope that loading order will be improved.

grant commented 5 years ago

I cannot debug your project's code. Please tell me the exact problem with ts2gas. There's a config in clasp to change the load order if you need that.

jakenjarvis commented 5 years ago

There seems to be a misunderstanding, but I created a sample repository just to report you this issue.

I know that filePushOrder exists in the config. It is actually specified in this sample, but it does not seem to work effectively. Also, when the number of files increases, manual priority setting is very troublesome.(I know I don't have to specify all the files)

ProjectAbnormal/.clasp.json

It looks like it is loaded in alphabetical order at runtime on GAS, regardless of the order in which Clasp pushes. In other words, class A is always load before class B. This is the issue. This issue is highlighted if inheriting classes and separating class definition files.

PopGoesTheWza commented 5 years ago

@jakenjarvis can you please share a link to the gas project that I can inspect?

Also I need to ask you, why do you use import and export statements? In a gas environment, export is only useful but when creating a gas library, while import can be useful to reference a *.d.ts which expose definition of a gas library.

If the purpose is to organise your gas code in separate namespaces, I would advise on using the namespace statements and forget the import/export approach.

jakenjarvis commented 5 years ago

@PopGoesTheWza Thank you for your reply. This sample presented by me is a simple test case. Please also check the README.

why do you use import and export statements?

I forgot the details, but I couldn't get it to work that way. Maybe my understanding was lacking, but the namespace statements were different from those of C# I had imagined. After all, when creating a large project, I could not determine which approach would be appropriate, and decided to use import/export statements. So far, no issues other than this issue have occurred.

The main reason I would like to use TypeScript is to do object-oriented programming. Managing files separately for each class is a common form. If I were presented with a sample of the recommended approach in this case before I used Clasp, I would not have got lost.

PopGoesTheWza commented 5 years ago

@jakenjarvis my knowledge of TypeScript for Google Apps Script is still empirical. Still, from my experience, export and import should be restricted to producing and consuming gas libraries respectively.

With a "standard" gas project with multiple *.ts files resulting in as many *.gs scripts, always bear in mind that all your script get added to the same global namespace (there is not distinct namespace per .gs file). To achieve some code containment, my experience is that the namespace statement bring the best solution with the less effort.

To illustrate export use, this project define a gas library. The exported statements (classes, enums, functions, variables) will exposed in the gas library namespace:

https://github.com/PopGoesTheWza/swgoh-help-api

To illustrate namespace usage as well as import (the latest being to correctly address exposed statements from the gas library):

https://github.com/PopGoesTheWza/swgoh-tb-sheets

All the above may or may not address your original issue. I will try to give a closer look later on.

jakenjarvis commented 5 years ago

Hi, @PopGoesTheWza

All the above may or may not address your original issue.

Using a namespace statement will probably not improve the issue. It's not a TypeScript issue, but a GAS load order issue. A solution is needed for either Clasp or ts2gas.

export and import should be restricted to producing and consuming gas libraries respectively.

I have long been familiar with object-oriented programming in other programming languages, and most of my source code is class definitions and Interface definitions. Even when all script files are added to the global namespace, code containment has already been achieved. So I'm not very worried about it.

I know this is changing the subject, I read the source code of your project lightly. There are many things to learn and it was very helpful. Thank you. Especially when I read this, I thought, "Oh, I see."

var client = new swgohhelpapi.module.exports.Client(settings);

I was a little surprised that I could access it this way, because I was not consciously reading at the source code after transpile. I did not think about this deeply.

var exports = exports || {};
var module = module || { exports: exports };

For me, this existed without thinking. XD If the Clasp README explains this, it may be more helpful. (Only I could not find it?)

jakenjarvis commented 5 years ago

Hi, @PopGoesTheWza @grant

I tried a sample that uses namespace statements, but still this issue is not solved. And I tried a couple of different ways of writing, but they all had the same issue. Please check each of the following. Please open each GAS script and execute myFunction in code.ts.

This proves that there is no problem in the way of writing.

Issue01

This is the first sample I posted.

ProjectNormal/src/AClass.ts

export class AClass {
  // (omission)
}

ProjectNormal/src/BClass.ts

import { AClass } from './AClass';
export class BClass extends AClass {
  // (omission)
}

Issue01-2

Created a project excluding import and export.

ProjectNormal/src/AClass.ts

class AClass {
  // (omission)
}

ProjectNormal/src/BClass.ts

class BClass extends AClass {
  // (omission)
}

Issue01-3

Created a project each was used with a separate namespace statement.

ProjectNormal/src/AClass.ts

namespace nameA {
  export class AClass {
    // (omission)
  }
}

ProjectNormal/src/BClass.ts

namespace nameB {
  export class BClass extends nameA.AClass {
    // (omission)
  }
}

Issue01-4

Created a project each was used with a same namespace statement.

ProjectNormal/src/AClass.ts

namespace nameAB {
  export class AClass {
    // (omission)
  }
}

ProjectNormal/src/BClass.ts

namespace nameAB {
  export class BClass extends nameAB.AClass {
    // (omission)
  }
}
PopGoesTheWza commented 5 years ago

Hello @jakenjarvis I had time to review your (nice) test code and here is my reasoning:

Thus my conclusion is:

@grant please review the above and advise. My opinion would be to open a @google/clasp issue referencing this one here (so that current work is not lost)

jakenjarvis commented 5 years ago

@PopGoesTheWza Thank you for your code review. I felt the issue was finally reported to you.

This is called hoisting and works as designed

Yes I know it. So I pointed out that the loading order is the issue.

after running clasp push, in the script editor, script files order do not comply with the filePushOrder directive:

I did not look at it carefully. If filePushOrder is not working properly, there was a issue of path interpretation, so it may be affecting. The reason I keep using Clasp1.6.3 is because Clasp does not work properly between 1.7.0 and 2.0.1 on Windows. (I have not tried Clasp 2.1.0 yet)

I posted the same issue here(Clasp#527). I was urged to instead Issue to ts2gas.

Currently this issue status is closed but it should be reopened.

PopGoesTheWza commented 5 years ago

Thanks @jakenjarvis I am not an expert on @google/clasp and merely just a contributor on ts2gas I did my tests with clasp 2.1.0 & ts2gas 1.6.1 and experienced the same mis-ordering issue.

jakenjarvis commented 5 years ago

Thanks @PopGoesTheWza This issue closes and continues with issue of Clasp.