Skip to content

[Feature] Add types#33

Open
lnfel wants to merge 2 commits into
ibrod83:masterfrom
lnfel:add-types
Open

[Feature] Add types#33
lnfel wants to merge 2 commits into
ibrod83:masterfrom
lnfel:add-types

Conversation

@lnfel

@lnfel lnfel commented Aug 26, 2023

Copy link
Copy Markdown

I hope this satisfies #32, only added types from what I can understand of the code at the moment. Will push updates as soon as I learn more about the structure of the data being used internally.

@ibrod83

ibrod83 commented Aug 26, 2023

Copy link
Copy Markdown
Owner

Thank you for the PR. I'll take a look at it later on

@ibrod83

ibrod83 commented Sep 22, 2023

Copy link
Copy Markdown
Owner

So I tested the code a bit, and it seems to me that are many types missing. For example, "CollectContent" class doesn't have any type associated with it.
Also, there are "incomplete types", like RootConfig:
export type RootConfig = { // Look at the pagination API for more details. pagination?: any; getPageData?: Function; getHtml?:Function // Receives a dictionary of children, and an address argument getPageObject?: Function; // Receives an axiosResponse object getPageResponse?: Function; // Receives htmlString and pageAddress getPageHtml?: Function; // Listens to every exception. Receives the Error object. getException?: (error: Error) => Promise<any>; } & HttpOperationConfig;
The "Function" type doesn't specify parameters and return value. This raises an error in my typescript client code.
Also, RootConfig seems to be missing "getHtml" hook.
What version and configuration of Typescript are you using? Perhaps it's less strict?

@lnfel

lnfel commented Sep 26, 2023

Copy link
Copy Markdown
Author

Yes, there are tons of missing types. I am using VSCode's bundled TS which is 5.2.2, maybe we could setup a tsconfig.json to explicitly include strict type checking. Gonna make some time for it this weekend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants