-
Notifications
You must be signed in to change notification settings - Fork 261
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
TypeScript declarations #98
Conversation
this uses the tsd-jsdoc npm package https://www.npmjs.com/package/tsd-jsdoc it gets a few errors and no idea how close the types are. Maybe someone more familar with typescript can take a look? checked in the generated types here You can run it with npm install ./node_modules/.bin/grunt jsdoc:ts
/** comment */ was being confused as a docstring and resulting in TS declaration pollution
it is worth saying that although this method is good at the initial stage, it does not give the desired severity of the types.
for
This case should be taken into account in the typing, which I showed in my example |
I think a good solution would be to generate files for each file in |
@NikitaIT The example you gave would be better solved by improving the
documentation rather than manually editing the definitions. See
TextureOptions for an example of how a SphereBufferOptions or similar type
could be represented in the docs.
However, I do not think that this is a blocking issue for this PR.
…On Sat, Jan 12, 2019, 7:01 PM Никита ***@***.***> wrote:
I think a good solution would be to generate files for each file in src
and correct them, and then make into Assembly by the ts compiler. Although
this will require a lot of effort, it will ease the work with d.ts in the
future.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#98 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AG7gbE8f8A5XCFCDFBSGntbscdnATQ-Dks5vChTMgaJpZM4Z8H28>
.
|
I incorporated this PR into #97 Your python translated to node/js is here Line 356 in 2085445
I tested it builds. I didn't test that it runs or that I translated every regex correctly. To build
To just build the type file
|
this doesn't feel right to me. To me they should be documented as returning a map of strings to WebGLBuffer like
|
Closing this PR; please see #102 instead |
Why? this makes the code more complex perhaps this description will look better, this is the option I showed in ts
|
@NikitaIT has a point. The more specific typing is better TypeScript.
However, I'd maintain that this is outside the scope of just getting
something that works into a release. It can be polished up to maximum
type-y goodness afterward.
…On Tue, Jan 15, 2019, 6:18 PM Никита ***@***.***> wrote:
Record<string, WebGLBuffer>
Why? this makes the code more complex
in ts you always rely on the types, and this type can not be relied on,
this at least contradicts the Occam's razor principle of good code
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#98 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AG7gbGwYvQOYgrkyPXx4_MuQRGqoU3_Vks5vDf88gaJpZM4Z8H28>
.
|
@pineapplemachine I'm not saying it's worth doing right now) but it's definitely worth it |
I'd like to understand the call for stronger types.
I have lots of C++/C# experience so I'm not allergic to types I don't really understand a hard coded type makes sense for attribute/buffer related things. it's perfectly valid to do this
Typing createSphereBuffers as returning some specific list doesn't make sense to me but maybe I don't understand the suggestion. In C++/C# making a hard coded type with 4 explicit proprties would mean adding a 5th property is prohibited. Is this different in typescript? All the functions that deal with buffers, attributes, and uniforms are explicitly string based maps to objects. There is no special meaning . You can make a shader
and then make arrays
In general Similarly you're equally free to do something like
or like above
I don't mind stronger types if typescript has some special way of saying "this thing is a map of strings to X and starts with these specific 4 things but youre free to add or remove more things to the map" but coming from C++/C# that can't be expressed there AFAIK so I'm assuming it also can't be expressed in typescript? |
@greggman There are tools to work around that sort of situation in TypeScript, since it's just JS and dynamic typing under the hood. Here is one option that I think could work better than simply interface ObjectWithAtLeastTheseProperties {
a: string;
b: string;
c: string;
[key: string]: string;
}
function doStuff() {
const abc: ObjectWithAtLeastTheseProperties = {
a: "apple",
b: "bear",
c: "clue",
};
abc.d = "door";
} |
Another option is leaving it up to the dependent code to specify if they want to add any more properties to the object or not. For example, you could strictly define the input type as was first discussed and then in cases where somebody wanted to mess with the object's properties, they'd write something more like this: const buffers = <[key: string]: WebGLBuffer> createSphereBuffers(...);
buffers.colors = someWebGLBuffer; Or for even more freedom, something like this: const buffers = <Object> createSphereBuffers(...);
buffers.colors = someWebGLBuffer;
buffers.whatever = "Hello, world!" |
See code not see where in it user-defined type of Line 643 in c69de9a
This is But user-defined type of Line 701 in c69de9a
or TUserArrayNames = keyof TUserArrayByName and user use like:
in c++ or c# u write code like
and user can create self enum or extends(i know what enum not extendable, but exists many ways escape this) |
This PR includes several documentation fixes that also fix TypeScript declaration issues.
I used this script to transform the grunt output into a syntactically valid TypeScript declaration file. I've tested to the extent that my project using a tiny slice of the API doesn't immediately die. Any further errors are probably mistakes in the documentation. For example, I only ran into
TextureOptions.color
incorrectly being marked as a mandatory attribute (when in fact it is optional) because I happen to be using TextureOptions in my small project that I used to help with testing, and I got a compilation error complaining that I hadn't included acolor
attribute.I'm not familiar with your build process @greggman so I'm gonna go ahead and request that you take this across the finish line, incorporating this script (...probably rewritten in JS. sorry, I'm faster at text processing in Python) as part of the TS declarations build process. You might also consider making some of these changes in the documentation directly, for example replacing "?" to document functions that accept an arbitrary type with "any"; and/or replacing "enum" with the perhaps more conventional "GLenum". (Right now, the script must change these things in the declarations file.)