Typescript definitions - Githubissues

mode777 commented 7 years ago

I wrote some typescript definitions for twgl, for a personal project. They don't cover the sub-modules yet, but I would be willing to add them. However, I don't know how (if at all) to contribute them. I could just create a separate repo and publish them to definitely-typed and "@ types/twgl.js" on npm but I feel like these repos/packages shouldn't be owned by me.

Any suggestions?

greggman commented 7 years ago

I'd love to take a look. I haven't dived into typescript yet. I'm curious how much verbose it makes things for stuff like twgl.createTextures

mode777 commented 7 years ago

The verbosity is managable if you use type inference where you can. Functions like twgl.createTextures will especially benefit from typescript, as you get full intellisense and type checking for all the options: untitled-1

The definitions themselves sure are verbose as all the types need to be delcared.

Currently I just keep them in my project repository, but I feel they should either be distributed with the main package or in a separate @types/twgl.js package (better choice IMO)

colorpump commented 7 years ago

Nothing happend? I haven't used yet twgl, but I would, if there were typescript definitions for it. I'd highly appreciate it.

mode777 commented 7 years ago

@colorpump : I'm still working on this. Twgl is a huge library, if you use all the sub modules. Here's a incomplete version, if you don't need the submodules

colorpump commented 7 years ago

@mode777 Thank you a lot. I forked it and looked a bit inside.

It seems to me actually almost complete. Maybe I can help. I just wondered how to handle the submodules:

In your file I found

declare module "twgl.js" {
    // ...

    // draw module
        export module draw {
            export function drawBufferInfo(gl: WebGLRenderingContext, bufferInfo: BufferInfo | VertexArrayInfo, type?: number, count?: number, offset?: number): void;
            export function drawObjectList(objectsToDraw: DrawObject[]): void;
        }

        export function drawBufferInfo(gl: WebGLRenderingContext, bufferInfo: BufferInfo | VertexArrayInfo, type?: number, count?: number, offset?: number): void;
        export function drawObjectList(objectsToDraw: DrawObject[]): void;

I couldn't find any best practice how to handle sub modules in .d.ts files.
Do you have any tipps how to elminate redundancy?
- otherwise it's also fine and I could continue in the same way

colorpump commented 7 years ago

@greggman back to mode777's original question :wink: ... Is it fine for you that he contributes the types to GitHub: DefinitelyTyped or would you like to do it as the owner of twgl.js?

Sorry for pushing so much :smile: ... I just think it would be a huge enhancement for twgl.js.

@mode777 I just don't really understand your point about 'owning' the repositories. Wouldn't you just fork DefinitelyTyped like 10 thousand others and make a pull request, and in the end it's part of @types? I'm sorry if I don't get something, I never contributed code by myself yet.

mode777 commented 7 years ago

@colorpump Not sure if I understand your question, but consuming the submodules will work just like in javascript:

import * as twgl from "twgl.js";

const info = twgl.primitives.create3DFBufferInfo(gl);

EDIT Sorry, now I get it: The redundancy here comes from twgl.js and is on purpose. That's why it is included twice.

@greggman: My opinion on this has changed. I think the definition files should be contributed together with the library in the same npm package. This seems to be the way many libraries are doing it (rxJS, electron, Angular...). If you agree, I will fork twgl.js add the definitions and submit a pull request.

greggman commented 7 years ago

I haven't used TypeScript yet. Looking forward to it!

Do one of you guys mind forking twgl, adding in the typescript stuff, and setting up whatever other files are needed so I can see it work and compile?

In other words I want to be able to, load up a typescript based example in VSCode, see VSCode show the typescript completions for twgl, see a twgl example written in typescript, see it process the sample into JavaScript so I can run it.

I don't know what the best way to do that is. In a subfolder? Stuff in the root? In a separate project?

If any of you have time to do that it would be really helpful. I'd basically like to see a working example so I have something to go off of.

colorpump commented 7 years ago

As mode777 took the initiative and also did already a lot, I want to let him go first.

@mode777 if you are busy these days, and want me to do it, it would be no problem for me - on the contrary: I would like to be helpful.

My idea would be:

to fork the main project and create a new branch called "typescript-definitions"
insert the file /src/twgl.d.ts
insert in /examples/: typescript-compiled.html, js/typescript-compiled.js and js/typescript.ts
- like examples/amd-compiled.html, examples/js/amd.js and examples/js/amd-compiled.js
modify package.json
- add "files": ["dist/3.x/twgl.d.ts"]

modify /Gruntfile.js

insert in copy: {... :

twgl.d.ts {src: ..., dest: `npm/base/${${verDir}}/twgl.d.ts` }

I don't think it's necesarry to have a typescript loader included. But if you think different, we have to ....

add to package.json "devDependencies":"ts-loader" (to compile the typescript example with webpack inside a Grunt task)
adjust the webpack config inside of Gruntfile.js that it builds the the typescript example as well when npm run build

I think it would be easier for people who are interested to install typescript globally and run on the personal computer: tsc or tsc -w to see the typescript example rebuilding.

What do you guys think?

mode777 commented 7 years ago

@colorpump This seems like a reasonable way to go. Ideally you would run npm install twgl.js, then import * from 'twgl.js' and get intellisense straight away. I think this should be the goal.

I'm actually quite busy at the moment, so if you want to finish the definitions (I just used the excellent documentation for it) it would be highly appreciated. Would you want to set up the fork as well?

@greggman I understand, that you want to see it work in action. I can set up a demo once the definitions are complete. Even plain Javascript users could benefit from the definitions, as many IDEs like vscode can use them to enchance dev experience.

colorpump commented 7 years ago

Yes, of course. I start tomorrow. I also set up the fork.

sami-badawi commented 7 years ago

I do not see the twgl.d.ts file anywhere. Do I need to run a gulp build myself to create this?

colorpump commented 7 years ago

@sami-badawi Currently it's only in my fork repo under the branch feature-typescript-support.

I realized that the twgl functions and Type Definitions are constantly under development. So I'm trying to work on an automatic test, in order to keep the definitions file in sync with the library itself.

I'll keep you updated here.

sami-badawi commented 7 years ago

Thanks @colorpump Last night I had made my own type definition based on work by @mode777 from this page https://gist.github.com/mode777/4f8eb25b123b2c5627034f797834f949 I placed it in: node_modules/twgl.js/index.d.ts This works but it seems a hackish and I am reluctant to check that into git my project: https://github.com/sami-badawi/shapelogic-typescript Is there a better way that I can take your file: https://github.com/colorpump/twgl.js/blob/feature-typescript-support/src/twgl.d.ts And use that instead? Where should I place twgl.d.ts ?

colorpump commented 7 years ago

I would place your definition file in a source folder and reference it by

///<reference path="./pathToYour/twgl.d.ts"/>
import * as twgl from 'twgl.js';

befefit: it's not in the node_modules, which will be overriden when npm install
disadvantage: you have to do this in every module, which has twgl.js as dependency.

To avoid a reference tag in every module, I think you can use the "files" property in tsconfig.json.

But you could also wait, until I'm done and until hopefully it's part of twgl.js. Then you will not need anything besides import * from 'twgl.js' ... I hope :wink:

sami-badawi commented 7 years ago

Thanks @colorpump. I followed your advice and added code.

///<reference path="./twgl.d.ts"/>
import * as twgl from 'twgl.js'

I am building with Webpack and got a compile error.

Cannot find type definition file for 'twgl.js'. (2688)

I changed the my code to:

///<reference path="./twgl.d.ts"/>
import '../node_modules/twgl.js/dist/3.x/twgl-full.js'
import * as twgl from 'twgl.js'

And still got same compile error.

Cannot find type definition file for 'twgl.js'. (2688)

Any ideas?

colorpump commented 7 years ago

I did the same thing, and it's working:

sami-badawi commented 7 years ago

Thanks @colorpump, my type definition is working fine, but I get a compile error when I call my build command:

webpack

When I installed the type definition file in node_modules/twgl.js/index.d.ts I got both working type definition and a working compiler. :D

Are you using Webpack too?

colorpump commented 7 years ago

yes, I tried the same example with webpack, and it did compile without errors.

I used webpack 2.4.1

But now I would say: You found a dirty solution. This should be enough until twgl.js comes with typescript definition. Once it's included, you won't have problems.

sami-badawi commented 7 years ago

I use Webpack 2.6, so I doubt that is the problem.

I found a slightly better solution. Instead of having the type definition live here:

node_modules/twgl.js/index.d.ts It lives here types/twgl.js/index.d.ts

And I have a npm task:

"type-copy": "cp -r types/ node_modules/"

If anybody else run into a similar problem before the twgl.js type definition gets rolled out, my code is here: https://github.com/sami-badawi/shapelogic-typescript/tree/feature/twgl

geon commented 6 years ago

@sami-badawi

I tried your type definition. I don't want it inside the node_modules folder, so I changed the first line from

declare namespace twgl {

to

declare module 'twgl.js' {

, removed the lines

export = twgl;
export as namespace twgl;

and added

{
    "compilerOptions": {
        "typeRoots": ["node_modules/@types", "./src/types"]
    }
}

to tsconfig.json.

geon commented 6 years ago

Any progress on this?

colorpump commented 6 years ago

sorry, I'm out, as I got other projects.

I wanted to write automated tests for the definition file, so that tests would bark when new updates in the core are done without updating the definition file. But that was a bigger pain as I expected.

I hope someone can implement this feature. Otherwise I hope to find some time for it again this year - but I cannot promise.

geon commented 6 years ago

FWIW, I am using the typings from @sami-badawi , with some tweaks. I added a couple of functions, and corrected the Arrays type.

https://github.com/geon/mem/commits/master/src/types/twgl.js/index.d.ts

greggman commented 5 years ago

I uploaded a new PR (just as an example). It's got @pineapplemachine 's PR in it. It also has a typescript example started. I don't know typescript at all and I don't feel like digging around trying to figure out how to get VSCode to see the type file when editing the example so I just pushed it here and maybe someone else more familiar with typescript can upload a working version.

What I want to happen

Load examples/textures-typescript.ts into VSCode. Hover mouse over twgl and see that VSCode see's it's the twgl module. Have it see other types etc...

What it does now

Hovering over twgl VSCode just says 'any'

Building

download the PR then

 npm install
 npm run build

Then, if you change something about how types.d.ts is made

 npm buildts   # remake dist/4.x/types.d.ts

To compile the .ts example

 ./node_modules/.bin/grunt ts   # build examples/textures-typescript.js from examples/textures-typescript.ts

The sample HTML file is in examples/textures-typescript.ts

Other issues

typescript by default has no knowledge of WebGL2 so loading types.d.ts into VSCode shows errors anywhere a webgl2 type shows up

greggman commented 5 years ago

Took @pineapplemachine 's latest and push a new PR here

https://github.com/greggman/twgl.js/pull/97

It builds and generates a .js file for the example, things seem to be typed in VSCode but the code doesn't run in the browser. It gets the error

textures-typescript.js:2 Uncaught ReferenceError: exports is not defined
    at textures-typescript.js:2

Searching online it's not entirely clear to me if I have to use webpack/babel/rollup to get it to work or what. Will look into it if/when I have time unless someone else knows how to fix it.

greggman commented 5 years ago

pushed 4.8.0 with .ts files

pineapplemachine commented 5 years ago

I updated the dependency version and it's working great with my TS project that prompted me to come and talk about definitions in the first place. Heck yeah. I will certainly fix any errors that I come across, and if you @ me in any issues created by other devs in regards to the definitions I can take a look at them too.

greggman commented 5 years ago

There's still the whole ArrayBufferView issue which I'm not 100% sure what I want to do there. The problem with ArrayBufferView is that DataView is an ArrayBufferView but it's not compatible with most of the places that take an ArrayBufferView. I declared TypedArray inside primitives.js but not sure if I should use that other places yet or not or just don't care that passing a DataView will fail

greggman commented 5 years ago

https://github.com/englercj/tsd-jsdoc/issues/89

greggman / twgl.js

Typescript definitions #50

What I want to happen

What it does now

Building

Other issues