lmcarreiro / ui5-typescript-example

a Master-Detail demo app (the same available in SAPUI5/OpenUI5 SDK) using TypeScript with npm ui5ts package
25 stars 7 forks source link

Better Typings - questions and feedback #1

Open apazureck opened 6 years ago

apazureck commented 6 years ago

Hi @lmcarreiro,

I wrote a parser to get better typings from the UI5 Documentation to use with your decorator.

check it out here: https://github.com/apazureck/UI5TypescriptDefinitionParser/tree/develop

I tried to use it with your repo. Thus, I forked it: https://github.com/apazureck/ui5-typescript-example

The fork contains the typings.

  1. I used declare module "foo" to be file system independent.
  2. I also tried to distinguish modules and globals to urge the programmer to use import foo from "bar" and get more stable code without missing modules. So maybe some globals are missing. I started with sap/ui/Global and declared everything there as module AND namespace.
  3. There are not much generics so far, I just added generic sap.ui.base.Event class and Model class, but more will follow when everything is working

So I have some problems:

The transpiler seems to create a wrong import in mockserver.ts. It imports import MockServer from "sap/ui/core/util/MockServer"; as MockServer_1 in javascript, but the MockServer_1 has this structure:

{
  default: {
    default: function (e,t,n) { ... }
    }
  }
}

Which causes the error:

TypeError: MockServer_1.default is not a constructor
mockserver.ts:30
    at Object.init (c:\dev\ui5-typescript-example\src\localService\mockserver.ts:30:23)
    at http://localhost:3000/:31:24
    at c:\dev\ui5-typescript-example\out\resources\sap-ui-core.js:88:2587
TypeError: Cannot read property 'Binding' of undefined
extensions::runtime:7
    at extensions::runtime:7:46
Error: "Natives disabled"
extensions::app:7
    at (anonymous) (extensions::app:7:22)
Error: "Natives disabled"
extensions::webstore:7
    at (anonymous) (extensions::webstore:7:22)

I used my gulp to automate the build. The output is in the out folder. Sources are still in the src folder. You should be able to use the example with vscode:

> npm install
> gulp

F5 will attach the debugger. Make sure the chrome debug extension is installed.

I do not understand properly what is happening in the factory and why there are two default exports, maybe you can help me understanding this problem and adjusting the typings accordingly?

Thanks & regards

apazureck commented 6 years ago

Oh and by the way I wanted to use export = in the first place, as I thought it should not return default on each import. I thought this would be closer to how it works in ui5. Sadly I did not get this to work either. Do you know a solution to this by chance? changing the factory did not help very much. The compiler still put in this:

var __importDefault = (this && this.__importDefault) || function (mod) {
    return (mod && mod.__esModule) ? mod : { "default": mod };
}

instead I wanted to have something like this:

var __importDefault = (this && this.__importDefault) || function (mod) {
    return mod;
}
lmcarreiro commented 6 years ago

I'll try to take a look at this code this weekend.

It is possible to use export = but you would need to change the define function.

lmcarreiro commented 6 years ago

I'm trying to run your code, but I'm getting other errors and can't debug it, you change the output folder and running in windows it can't load the *.ts from the source maps...

But I understood better what you are trying to do...

Since the define() function doesn't know which of the following forms of import you are using...

import { support } from "sap/ui/Device";
// ...
} else if (!support.touch) { // apply "compact" mode if touch is not supported
import Device from "sap/ui/Device";
// ...
} else if (!Device.support.touch) { // apply "compact" mode if touch is not supported

The first one if compiled to:

else if (!Device_1.support.touch) {

And the second to:

else if (!Device_1.default.support.touch) {

I'm using this code to call the factory:

vFactory(null, exports, ...args.map((d: any) => ({ default: d })));

I think that the only way to get both of them working at the same time, is to pass to the factory something like this:

 function define(aDependencies: string[], vFactory: (...args: any[]) => any): any
 {
     //remove the dependencies "require" and "exports" generated by typescript compiler
     var newDependencies = aDependencies.slice(2);

     //overrides the typescript generated factory, passing to the original factory:
     // - null instead of the dependency "require"
     // - a new object with an empty "default" property as "exports"
     //and returns the default export of the typescript generated module
     var newFactory = (...args: any[]) => {
         var exports: { default: any } = { default: undefined };
-        vFactory(null, exports, ...args.map((d: any) => ({ default: d })));
+        vFactory(null, exports, ...args.map((d: any) => {
+            if (!d.hasOwnProperty("default"))
+                Object.defineProperty(d, "default", { value: d });
+            return d;
+        }));
         return exports.default;
     };

     //call the original sap.ui.define() function, with adapted dependencies and factory
     return sap.ui.define(newDependencies, newFactory);
 }

This would handle the imports only. To handle the exports too, I think that it would work:

 function define(aDependencies: string[], vFactory: (...args: any[]) => any): any
 {
     //remove the dependencies "require" and "exports" generated by typescript compiler
     var newDependencies = aDependencies.slice(2);

     //overrides the typescript generated factory, passing to the original factory:
     // - null instead of the dependency "require"
     // - a new object with an empty "default" property as "exports"
     //and returns the default export of the typescript generated module
     var newFactory = (...args: any[]) => {
-        var exports: { default: any } = { default: undefined };
-        vFactory(null, exports, ...args.map((d: any) => {
+        var exports: any = {};
+        var result = vFactory(null, exports, ...args.map((d: any) => {
             if (!d.hasOwnProperty("default"))
                 Object.defineProperty(d, "default", { value: d });
             return d;
         }));
-        return exports.default;
+        return result || exports;
     };

     //call the original sap.ui.define() function, with adapted dependencies and factory
     return sap.ui.define(newDependencies, newFactory);
 }
lmcarreiro commented 6 years ago

I've published that change at version 0.4.0-dev.20180312, could you test if it resolves your problem?

You can install it with: npm install ui5ts@next

This change should allow you to import using import { support } from "sap/ui/Device"; and to export using export =

apazureck commented 6 years ago

Hi @lmcarreiro,

Thank you for the quick feedback. I'll give it a try today!

You should be able to debug when you use chrome and hit refresh (CTRL R) once. I did not get your bsync to work :(, so I used the template of my project with gulp. And I like to have a seperate out folder to have all the "junk" separated from my sources - and it was configured in my template :)

Export

I am currently reconfiguring the parser to put out all modules as export =

I thought in UI5 you have one main export (normally the first one in the api.json) and the other exports are stacked on that. So I thought this would be the closest to express the module loading and, thus, to make the code also usable in javascript without more knowledge about typescript.

Here some examples from my experience:

Export default usage is more convenient in typescript, as you don't have to think about overloading:

// module1.ts

export default Class extends Baseclass {
...
// namespacy stuff
static sfunc(): void {
}
}

// additional classes or enums
export Class2 {
}

export enum Enumeration {
}

//module2.ts
import Class from "my/base/namespace/module1";

// Will be resolved to (if I am not mistaken)
define(["my/base/namespace/module1"], function(import1) {
var Class_1 = import1.default;
}

Export equals

//module1.ts
export Class;
class Class {
}
namespace Class {
// enum and such has to go here
}

//module2.ts
import Class from "my/base/namespace/module1";

// Will be resolved to
define("moduleimport...", function(Class) {
}

And in both cases you could do:

import { Enumeration } from "my/base/namespace/module1";

What do you think about that?

lmcarreiro commented 6 years ago

I forgot that you used gulp and tried to run with npm start. You could make the start npm script to call the gulp instead of bsync+tsc.

I didn't understand this last post, wouldn't you use export = Class; instead of export Class;

apazureck commented 6 years ago

Hi,

sorry for the delay. I tried to figure out, if we can solve the problem you described in your answer.

The first one if compiled to: else if (!Device_1.support.touch) { And the second to: else if (!Device_1.default.support.touch) {

I thought if you export it via export = you wouldn't have to distinguish these cases. You should not get a default export, do you?

For example:

So if you create a module in typescript you might want to share it with your colleagues, who are using javascript. They would not get the expected import using sap.ui.define if you use export default. You would have to use export = to get modules similar to the sap modules.

I have hardly any knowledge on module import and export and how it works in javascript, so I hope you could help figuring out, how the declarations are done best to reflect the ui5 framework in typescript.

I hope I can try your code this weekend.

lmcarreiro commented 6 years ago

Hi @apazureck I saw that discussion at the SAP/openui5#88 and I was wondering... Have you tried that new dev version that I published (0.4.0-dev.20180312)? Could you try it using npm install ui5ts@next ? It should work with export =.

apazureck commented 6 years ago

Hi @lmcarreiro,

I started to have a look at it, but I still used the export default typings. So that was the error :). I did want to have a closer look, as I got some weird behavior and could not get the breakpoints to work properly (did not execute the define function). So I wanted to figure out the reason for that first before contacting you, but as I see now it would have been better to ask first...

Sadly I did not commit my export = declarations, so I do have to do them again. I opened a branch for that: https://github.com/apazureck/UI5TypescriptDefinitionParser/tree/feature/ExportEquals I hope I can continue the work on that the next days and will give the whole thing a try. Would be great if it works. Thank you for your help.