search

create3000 / x_ite

X_ITE X3D Browser, view and manipulate X3D and VRML scenes in HTML.
https://create3000.github.io/x_ite/
Other
67 stars 15 forks source link

TypeScript types for x_ite #152

Closed gwhitney closed 1 year ago

gwhitney commented 1 year ago

Given the number of folks that are using TypeScript these days, you might want to provide TypeScript typings for x_ite. The ideal would be for them to be automatically generated, but I am not sure how that would be accomplished (I tried checking out the repository and running tsc --declaration --allowJs but that failed badly). So another plan would be to hand-write a TypeScript .d.ts file with type declarations. I attach my initial effort at a comprehensive version of this, in case it will be of help. It surely is incomplete/has errors, but it does compile, and it works for the limited number of calls I am making at the moment.

One possibility is I could submit a PR to @DefinitelyTyped, but it would make more sense for the maintainers of this package to supply the typings. (And DefinitelyTyped does point out that it's clearer for packages to supply the types alongside their JavaScript packages.) I could also submit a PR with this .d.ts file, but as it is hand written it would not be clear to me where it should go so that it would be properly packaged with x_ite so that others using TypeScript could take advantage of it effortlessly.

Thank you for creating x_ite -- if I can be of any further assistance with providing TypeScript typings, please just let me know. x_ite.d.ts.txt

create3000 commented 1 year ago

https://github.com/create3000/x_ite/issues/116

create3000 commented 1 year ago

I'm not that familiar with TypeScript, which means I can't maintain the declarations file either, which means a PR is always welcome.

create3000 commented 1 year ago

I have already had a look at the .ts file, there are still a few small errors in there, but on the whole it already looks very good.

create3000 commented 1 year ago

I put the .d.ts file in the dist folder https://github.com/create3000/x_ite/tree/development/dist. I hope this is the right place. Could you please give me an example how I can benefit from this file.

gwhitney commented 1 year ago

Well the problem is I am no expert on creating .d.ts files for javascript projects, either. When you make a native TypeScript project, it just generates the .d.ts for you.

But I think that you have put the .d.ts file in the best place, right alongside the module file. And when it is there, one then can/should unwrap it from the declare module URL { ... } block and move everything inside up one level. Also I think the line const X3D : X3D should actually be declare const X3D : X3D.

I think with those changes if it is published this way on npm and imported into a typescript project, then one can import X3D from 'x_ite' and start calling the functions and they will just typecheck properly. I will make a dummy TypeScript project and install the current x_ite and just put the file modified in this way by hand alongside the module file in node_modules and see if it works and let you know.

gwhitney commented 1 year ago

Yes, indeed, that worked, with the one additional change that I had to add a declare before class X3DField. The exact file I added in dist is attached to this message, to avoid ambiguity.

So if you ship this file alongside the x_ite JavaScript files, then it should be usable directly from TypeScript, and intellisense should work (although I don't use an IDE, so I can't vouch for that completely). I would recommend it in the next release. Let me know if there is more I can do to help. (And of course, yes, I am sure some improvements to this .d.ts will be needed over time, but I hope it is an OK place to start.) x_ite.d.ts.txt

create3000 commented 1 year ago

I have tested the new file with VSCode and IntelliSense works fine for both JS and TS files. Thanks for your great efforts. If you have further updates, please let me know.

gwhitney commented 1 year ago

Well once it's in an x_ite published release, then thereafter I can just file PRs to this repository with any corrections/improvements that I may find. Thanks!

create3000 commented 1 year ago

I can make a release next Sunday if all is fine, and we have still time for changes.

gwhitney commented 1 year ago

OK, I have updated my project to use the .d.ts directly from the develop branch, and it is working OK at the moment. If I discover anything that seems to need a change, I will just file a PR against that branch. Once it's published, I will switch to using it from the main branch, or just grab it from the CDN if it ends up on there too (not sure if it will).

create3000 commented 1 year ago

I have added you to the list of contributors in https://github.com/create3000/x_ite/blob/development/package.json, for that reason it is good idea to make a PR when the file has changed, because GitHub automatically keeps track of it.

create3000 commented 1 year ago

Version 8.12.1 is out now. Please drop me a note if it is working for you.

gwhitney commented 1 year ago

Oh dear, seems I overlooked responding -- yes, I am compiling and running fine with the latest version, now 8.12.4. Thanks!