Skip to content
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

Add default values to TypeScript declaration files #19919

Closed
dmnsgn opened this issue Jul 24, 2020 · 11 comments · Fixed by #20036
Closed

Add default values to TypeScript declaration files #19919

dmnsgn opened this issue Jul 24, 2020 · 11 comments · Fixed by #20036

Comments

@dmnsgn
Copy link
Contributor

dmnsgn commented Jul 24, 2020

Description of the problem

Having definitions helps a lot but they are lacking the proper setting of the default value. It is usually in the comment just above each property so is there any blocker to include them as well.

See for instance:

/**
* If autoClear is true, defines whether the renderer should clear the color buffer. Default is true.
*/
autoClearColor: boolean;
/**
* If autoClear is true, defines whether the renderer should clear the depth buffer. Default is true.
*/
autoClearDepth: boolean;
/**
* If autoClear is true, defines whether the renderer should clear the stencil buffer. Default is true.
*/
autoClearStencil: boolean;

could be:

export class WebGLRenderer implements Renderer {
// ...
 	autoClearColor: boolean = true; 
 	autoClearDepth: boolean = true; 
 	autoClearStencil: boolean = true; 
// ...
}

It shouldn't be an issue since types are actually exporting classes and not just declareing them and could help parsing the codebase and monitor default allocations.

Three.js version

N/A

Browser

N/A

OS

N/A

Hardware Requirements (graphics card, VR Device, ...)

N/A

@gkjohnson
Copy link
Collaborator

It doesn't seem this is possible unless I've done something wrong. Adding a default value onto a class member gives the following error when running tsc:

error TS1039: Initializers are not allowed in ambient contexts.

And adding one into a function signature errors with:

error TS2371: A parameter initializer is only allowed in a function or constructor implementation.

@donmccurdy
Copy link
Collaborator

Yeah, I don't think TS typings (as opposed to source code) allow this.

@dmnsgn
Copy link
Contributor Author

dmnsgn commented Jul 30, 2020

I see. As an alternative, would you consider adding the default value as a JSDoc comment:

/**
 * @param {string} [somebody=John Doe] - Somebody's name.
 */

Or any consistent way to parse the default values across the codebase?
I am not advocating TS or JSDoc in any way, just trying to make it easier and more accessible for anyone to understand what default values. Application could be both useful in the documentation and for codebase analysis purpose.

@Mugen87
Copy link
Collaborator

Mugen87 commented Jul 30, 2020

As an alternative, would you consider adding the default value as a JSDoc comment:

It was decided in #19940 not to do this.

@dmnsgn
Copy link
Contributor Author

dmnsgn commented Jul 30, 2020

Including the information in the .js file was decided against yes, but what about having the default value in the declaration files since they are manually handled? Type information is already present in the declaration so it could be simply a @default tag or anything you judge suitable for parsing.

Using my previous example:

export class WebGLRenderer implements Renderer {
// ...
        /** 
 	 * If autoClear is true, defines whether the renderer should clear the color buffer.
         * @default true
 	 */ 
 	autoClearColor: boolean;
// ...
}

@Mugen87
Copy link
Collaborator

Mugen87 commented Jul 30, 2020

Well, this seems reasonable to me 👍 .

@mrdoob
Copy link
Owner

mrdoob commented Jul 30, 2020

We just need someone to offer maintaining them.

@dmnsgn is that something you would like to do?

@donmccurdy
Copy link
Collaborator

donmccurdy commented Jul 30, 2020

Just to confirm here — do VSCode and other editors display JSDoc on type defs, as a fallback to JSDoc on the source itself? I'm guessing that's true because the TS compiler copies JSDoc from source TypeScript to the type definitions when it compiles TS->JS, but I'm not really sure about how editors decide what to use.

@donmccurdy donmccurdy reopened this Jul 30, 2020
@dmnsgn
Copy link
Contributor Author

dmnsgn commented Aug 3, 2020

I might be able to update the initial comment definitions and PR, then I assume if defaults are changed for some reason in the js source files, the ts declaration files should be checked in parallel with the PR or when merging.


@donmccurdy just tried in VSCode and it does: if I update WebGLRenderer.d.ts in node_modules/three with @default true:

        /**
	 * If autoClear is true, defines whether the renderer should clear the color buffer. Default is true.
         * @default true
	 */
	autoClearColor: boolean;

it will appear:

Screenshot 2020-08-03 at 16 57 02

@mrdoob
Copy link
Owner

mrdoob commented Aug 3, 2020

Sorry, if there is no maintainer taking care of this it will then have to be me. And I'm unable to maintain more things.

@dmnsgn
Copy link
Contributor Author

dmnsgn commented Aug 7, 2020

Alright @mrdoob, I have opened the PR here: #20036. For maintaining it, I guess you can tag me in the future if defaults changes and these needs to be updated?

Warggr referenced this issue in Warggr/petri-js Mar 12, 2024
- Now partially in Typescript
- Dependencies updated
- Split between model, view, and controller
- WIP: controller not implemented yet
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants