espruino / EspruinoTools

JavaScript library of tools for Espruino - used for the Web IDE, CLI, etc.
Apache License 2.0
150 stars 89 forks source link

Better typescript type support #175

Open 2234839 opened 7 months ago

2234839 commented 7 months ago

I'm a newbie to esprunio and the official documentation [https://www.espruino.com/Typescript+and+Visual+Studio+Code+IDE](https://www.espruino.com/Typescript+ and+Visual+Studio+Code+IDE) which seems to be aged and actually difficult to run.

The library @types/espruino also doesn't type-define as expected, so I also need to write a .d.ts file for more accurate type-definition when using typescript. https://github.com/2234839/espruino_ts

I remember seeing an issue about espruino automatically generating type definitions, and I'd like to handwrite a better @types/espruino for esprunio if this converter doesn't live up to expectations.

Is this required, and if so what are the other requirements?

gfwilliams commented 7 months ago

Hi,

Thanks - yes, the documentation for TypeScript on Espruino isn't great

@bobrippling when you have a moment, would you be able to jump in and provide some pointers? @bobrippling has done a huge chunk of Typescript work, especially for Bangle.js, and I believe most of that is now auto-generated: https://github.com/espruino/BangleApps/tree/master/typescript

bobrippling commented 7 months ago

Hey - yes I'm not familiar with @types/espruino but I'd be happy to take a look over PRs to update it if that's what you're thinking? Let's go through the problems you found when trying to run it too, we can see what we can improve there.

How have you found setting up the repo you referenced?

gfwilliams commented 7 months ago

Actually I realise the directory I linked also links to https://github.com/espruino/Espruino/blob/master/TYPESCRIPT.md which explains about auto-generating the typescript - a lot of work has gone into that, so if we can find any way to not hand-write TS declarations that would be way better.

I feel like probably we need a generic (non-bangle.js) export and to then try and get @types/espruino updated, ideally with some instructions in there pointing back to https://github.com/espruino/Espruino/blob/master/TYPESCRIPT.md to that folks in future know how to update it

2234839 commented 7 months ago

I tried updating scripts/build_types.js so that the espruino.d.ts it generates matches the type definitions (http://www.espruino.com/Reference#software) in the official api documentation

bobrippling commented 7 months ago

Cool, do you have the branch for this?

2234839 commented 7 months ago

Cool, do you have the branch for this?

I am reading through the code; I find that scripts/build_types.js generates much better type files than the npm lib @types/espruino, so why hasn't @types/espruino been updated. In addition, I can see that some of the type definitions could be optimized, for example, function getDetails(callback?: any): any; The callback here could be better defined, so that the user knows the type of the callback function's arguments.

gfwilliams commented 7 months ago

so why hasn't @types/espruino been updated.

That's a good question - maybe you could submit a PR to the project?

I can see that some of the type definitions could be optimized

If you see something you want changing you are welcome to contribute updated typings. If you find the relevant file (which you can do by going to https://www.espruino.com/Reference#software, finding the function and clicking => in the title) you can add typescript as mentioned in https://github.com/espruino/Espruino/blob/master/TYPESCRIPT.md to improve the automatic generation.

2234839 commented 7 months ago

I'm writing a simple type inference script, and during debugging I realized that JsVar *jswrap_wifi_getDetails(JsVar *jsCallback) {{ Is the {{ here a clerical error, when in fact only a { is needed (This is inferred from my experience with js.)

图片

gfwilliams commented 7 months ago

I don't see any `{{ in that screenshot? But that file is C, not JS

2234839 commented 7 months ago

I don't see any `{{ in that screenshot? But that file is C, not JS

https://github.com/espruino/Espruino/blob/62546cca3a8cebeee7ccb6fd7a4a89d2b973e61d/libs/network/esp8266/jswrap_esp8266_network.c#L742C1-L742C52

The end of this line

gfwilliams commented 7 months ago

Ahh, sorry - didn't spot that! Yes, that's a typo. It won't affect the code, but it's a bit ugly for sure - just fixed it