Add all typescript types for web-js #68
Conversation
There was a problem hiding this comment.
@mfehr94 Thx for the pull request. I think in some files the jsdocs is missing and so some any was added to some methods. We should where possible fix the jsdocs and update the typescript types. Also analyse why some .d.ts seems to have some methods or props just missing.
tsconfig.json
Outdated
| { | ||
| "include": ["packages/**/*"], | ||
| "compilerOptions": { | ||
| "allowJs": true, | ||
| "declaration": true, | ||
| "emitDeclarationOnly": true, | ||
| "outDir": "types", | ||
| "declarationMap": true | ||
| } | ||
| } |
There was a problem hiding this comment.
| { | |
| "include": ["packages/**/*"], | |
| "compilerOptions": { | |
| "allowJs": true, | |
| "declaration": true, | |
| "emitDeclarationOnly": true, | |
| "outDir": "types", | |
| "declarationMap": true | |
| } | |
| } | |
| { | |
| "include": ["packages/**/*"], | |
| "compilerOptions": { | |
| "allowJs": true, | |
| "declaration": true, | |
| "emitDeclarationOnly": true, | |
| "outDir": "types", | |
| "declarationMap": true | |
| } | |
| } |
types/components/expand/expand.d.ts
Outdated
| @@ -0,0 +1,5 @@ | |||
| declare function _exports(): { | |||
| initialize: (el: HTMLElement, options: object) => void; | |||
There was a problem hiding this comment.
Is there a way we can provide a interface for this? So we don't need declare this over again?
types/services/api/api.d.ts
Outdated
| @@ -0,0 +1,6 @@ | |||
| export declare function get(uri: any, data: any): Promise<any>; | |||
There was a problem hiding this comment.
uri: any looks false, same for data: any
| @@ -0,0 +1,5 @@ | |||
| export const registerComponent: any; | |||
There was a problem hiding this comment.
this looks weird?
Why looks this different then the core.js?
There was a problem hiding this comment.
Because of this declaration:
var lazy = {
componentRegistry: {},
serviceRegistry: {},
deferredComponents: {},
deferredServices: {},
};| @@ -0,0 +1,14 @@ | |||
| declare namespace web { } | |||
| import registerComponent = web.registerComponent; | |||
There was a problem hiding this comment.
Does typescript correctly read the types from jsdocs here?
Lines 184 to 186 in 49f6e9e
So it will correctly fail if it is called with something else?
| @@ -0,0 +1,3 @@ | |||
| declare function _exports(func: any, wait: any, immediate: any): (...args: any[]) => void; | |||
There was a problem hiding this comment.
jsdocs should be defined correctly so it will also get func: any, wait: int correctly:
web-js/packages/services/debounce/debounce.js
Lines 6 to 8 in 49f6e9e
| initialize: (el: HTMLElement, options: object) => void; | ||
| }; | ||
| export = _exports; | ||
| //# sourceMappingURL=accordion.d.ts.map |
There was a problem hiding this comment.
Do we need the map files?
There was a problem hiding this comment.
@alexander-schranz if you want to provide autocomplitons for IDE yes, if not it can be disabled with "declarationMap": false
| @@ -0,0 +1 @@ | |||
| {"version":3,"file":"accordion.d.ts","sourceRoot":"","sources":["../../../packages/components/accordion/accordion.js"],"names":[],"mappings":"AAIiB;qBAsCF,WAAW,WACX,MAAM;EA0EpB"} | |||
There was a problem hiding this comment.
Are this files required?
There was a problem hiding this comment.
@alexander-schranz if you want to provide autocomplitons for IDE yes, if not it can be disabled with "declarationMap": false
| export const key: string; | ||
| export const promise: any; |
There was a problem hiding this comment.
For me looks here are all methods missing.
|
@alexander-schranz thank you for the first review and the suggestions. This are autogenerated types from typescript compiler and just a quick starting point for me to work on. I will check the JSCodes and try to fix them, also adding the missing props and methods as soon as possible. |
|
@mfehr94 Do we even need the |
|
@alexander-schranz good possible, I must test that and add the missing JSDocs. At the moment type script don't like the import of the web in custom JS components in my test project. |
packages/experimental/lazy/lazy.js
Outdated
| deferredComponents: {}, | ||
| deferredServices: {}, | ||
| }; | ||
| var lazy = {}; |
There was a problem hiding this comment.
| var lazy = {}; | |
| var lazy = { | |
| componentRegistry: {}, | |
| serviceRegistry: {}, | |
| deferredComponents: {}, | |
| deferredServices: {}, | |
| }; |
This object should stay as before.
closes #67