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

A way to specify class properties using JSDoc #26811

Open
4 tasks done
arendjr opened this issue Aug 31, 2018 · 22 comments
Open
4 tasks done

A way to specify class properties using JSDoc #26811

arendjr opened this issue Aug 31, 2018 · 22 comments
Labels
Domain: JavaScript The issue relates to JavaScript specifically Experience Enhancement Noncontroversial enhancements Suggestion An idea for TypeScript
Milestone

Comments

@arendjr
Copy link

arendjr commented Aug 31, 2018

Search Terms

JSDoc class properties any key @property

Suggestion

I would like to be able to extend the type of an ES6 class using JSDoc comments, such that TypeScript understands there are other properties that might be added to the object later.

Alternatively, I would like a way to indicate that a given class acts as a container that can contain any possible key.

Use Cases

I have a class that is used like a DTO. It has some properties that are guaranteed to be there, but is also used for a lot of optional properties. Take, for example:

class DTO {
    constructor(id) {
         /**
          * @type {string}
          */
         this.id = id;
    }
}

TypeScript now recognizes the object type, but whenever I try to use other properties, it complains. Currently, I'm resorting to something like this as a work-around:

class DTO {
    constructor(id) {
         /**
          * @type {string}
          */
         this.id = id;

         /**
          * @type {Object?}
          */
         this.otherProperty;
    }
}

But it's ugly and verbose, and worse, it includes actual JavaScript code, that serves no purpose other than to provide type information.

Examples

Rather, what I would like to do is something like this (that I would like to be equivalent to the snippet above):

/**
 * @property {Object} [otherProperty]
 */
class DTO {
    constructor(id) {
         /**
          * @type {string}
          */
         this.id = id;
    }
}

Another equivalent alternative could be (but currently doesn't compile because of a "Duplicate identifier" error):

/**
 * @typedef {Object} DTO
 * @property {string} id
 * @property {Object} [otherProperty]
 */
class DTO {
    constructor(id) {
         this.id = id;
    }
}

Or, otherwise, some way to indicate this class can be extended with anything. Which means I would like a way to specify the following TypeScript using JSDoc and I want it to apply to an existing ES6 class (because I do use the class to be able to do instanceof checks):

interface DTO {
    id: string,
    [key: string]: any,
}

Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript / JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. new expression-level syntax)
@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript Experience Enhancement Noncontroversial enhancements labels Sep 17, 2018
@RyanCavanaugh RyanCavanaugh added this to the Future milestone Sep 17, 2018
@RyanCavanaugh
Copy link
Member

@sandersn can this be accomplished with declaration merging today?

@sandersn sandersn added the Domain: JavaScript The issue relates to JavaScript specifically label Oct 18, 2018
@sandersn
Copy link
Member

Is the ideal solution a list of possible properties, or can the DTO truly hold any property that will come along?

There are a couple of workarounds you can use:

  1. As @RyanCavanaugh suggested, you can create a d.ts that merges with DTO:
interface DTO {
  [key: string]: any
}

Merging happens automatically in scripts (since they're all global), or you will have to wrap the interface in declare module "mymodule" { ... } if you're using ES6 modules.

As you point out, @typedef does NOT merge with classes, because it creates a type alias, not an interface.

  1. If you can use ES Next features, property declarations would solve the individual property list problem:
class C {
  /** @type {Object?} */
  otherProperty;
  constructor(id) {
    this.id = id;
  }
}

This feature isn't done yet either, but is tracked by #27023.

@arendjr
Copy link
Author

arendjr commented Oct 19, 2018

I'm sorry, I'm probably missing something. If I create a .d.ts file with the same basename as the .js file, it seems to shadow it, so I don't think that's what I should be doing. But adding it with a different basename it seems to have no effect at all, I don't see any "automatic merging" happening (I am using CommonJS indeed) and I don't know how I would need to import the interface otherwise.

I'm not that well-versed in TypeScript, so I'm feeling a little lost here...

@sandersn
Copy link
Member

Yes, you are correct, X.d.ts and X.js causes X.d.ts to shadow the JS file. If you are using a tsconfig.json (or jsconfig) then you need to make sure both the js and d.ts are included in the files list or glob. Otherwise, you'll need an explicit <reference path="Y.d.ts" /> inside of X.js.

I'm not actually sure whether CommonJS needs to use module augmentation syntax in order to merge with .d.ts interfaces. Let me check...

@sandersn
Copy link
Member

sandersn commented Oct 19, 2018

Bad news. CommonJS modules do indeed require matching module augmentation syntax, but it does not work right now. Here's what I got to work in Typescript:

// @Filename: ex.d.ts
export var dummy: number;
declare module "./welove" {
  interface DTO {
    // (number is more obvious when it works)
    [s: string]: number 
  }
}
// @Filename welove.ts
export class DTO {
  constructor(public n: number) { }
}
var c = new DTO(1)
c.n // ok
c.w // ok

Unfortunately, the same thing doesn't work with commonjs:

// @Filename: ex.d.ts
export var dummy: number;
declare module "./test" {
  interface DTO {
    // (number is more obvious when it works)
    [s: string]: number 
  }
}
// @Filename test.js
class DTO {
  /** @param {number} n */
  constructor(n) { 
    this.n = n
  }
}
var c = new DTO(1)
c.n // ok
c.w // error
module.exports = { DTO }

I'll file a bug to make sure this works.

At this point, I think we have 3 proposals to solve this problem. Unfortunately, none of them work right now!

@sandersn sandersn reopened this Oct 19, 2018
@sandersn
Copy link
Member

sandersn commented Oct 19, 2018

Never mind. Thanks to @andy-ms for pointing out that the distinction is not TS vs JS, but ES exports vs CommonJS value exports. That is, this works:

// @Filename: ex.d.ts
export var dummy: number;
declare module "./test" {
  interface DTO {
    [s: string]: any
  }
}
// @Filename test.js
module.exports.DTO = class {
    /** @param {number} id */
    constructor(id) {
        this.id = id
    }
}
var c = new module.exports.DTO(1);
c.id  // ok
c.w // ok

But my previous example, with module.exports = { DTO }, does not. But it also doesn't work in TS, where you use the syntax export = DTO. That's because it creates an object literal and the compiler only knows how to augment modules, not object literals.

@weswigham weswigham added the In Discussion Not yet reached consensus label Nov 6, 2018
@RyanCavanaugh RyanCavanaugh removed the In Discussion Not yet reached consensus label Jul 29, 2019
@zzswang
Copy link

zzswang commented Jul 9, 2020

any update on this?

want to use jsdoc to add optional prop of a class, currently it does't respect optional flag.
the prop other will be recognized as a required prop.

class DTO {
         /**
          * @type {string?}
          */
          other;
}

@thw0rted
Copy link

thw0rted commented Jul 9, 2020

The JSDoc @type docs say a "nullable type" is {?string} not {string?}. Does it work with the nullable syntax?

@ExE-Boss
Copy link
Contributor

ExE-Boss commented Jul 9, 2020

Originally posted by @zzswang in #26811 (comment)

want to use jsdoc to add optional prop of a class, currently it does't respect optional flag.
the prop other will be recognized as a required prop.

You need to write:

class DTO {
	/**
	 * @type {string | undefined}
	 */
	other;
}

Note that class instances will have the other property, since it’s initialised to undefined.

@ekr3peeK
Copy link

Is it possible somehow to make this work with ES6 export/imports? What I am trying to achieve, is to "extend" an exported class JavaScript class, with overriden methods from a d.ts file, so that IntelliSense would give me suggestions on both, and while what I am trying works as expected when I am on the global scope (ie, with files that aren't modules), I can't seem to get this to work with modules.

This is what I am trying:

// test.js

/// <reference types='./custom' />

class MyTestClass {
    constructor() {

    }

    inlineMethod() {
        
    }
}

export default MyTestClass;
// custom.d.ts
module "./test" {
    interface MyTestClass {
        public tsMethod():any;
    }
}

If I omit the module from the d.ts file, and declare global, and remove the export from the js file, IntelliSense suggests both the tsMethod and the inlineMethod for me, but I can't get this to work with modules.

@miyasudokoro
Copy link

Here is my use case, which involves somersaults through the hoops of making good JSDocs for Proxy declarations in pure JavaScript.

The problem here is that I need the following things to work simultaneously:

  1. @param {MyClass} for JSDoc
  2. @param {MyClass} for TypeScript
  3. instanceof MyClass
  4. Istanbul code coverage

Class constructor version

/** @class MyClass
 * @property email {string} The user's email
 * @property name {string} The user's name
 */
export default class MyClass {
    constructor( target ) {
        return new Proxy( target, proxyHandler ); // you can use "new MyClass"
    }
}
const proxyHandler = {
    // lots of stuff here
    getPrototypeOf: () => MyClass.prototype // you can use "instanceof MyClass"
}

// ... later in another file ...
import MyClass from './MyClass.js';

/** @param {MyClass|OtherClass} the data
 */
getUserEmail( myInstance ) {
    if ( myInstance instanceof MyClass ) {
        return myInstance.email;
    }
}

Factory version using empty class

/** @class MyClass
 * @property email {string} The user's email
 * @property name {string} The user's name
 */
export default function MyClass() {} // no "new MyClass" in this implementation

/** Factory for MyClass proxy.
 * @returns {MyClass}
 */
MyClass.Factory = function( target ) {
    return new Proxy( target, proxyHandler );
}
const proxyHandler = {
    // lots of stuff here
    getPrototypeOf: () => MyClass.prototype // you can use "instanceof MyClass"
}

// the use in the other file is same as class constructor version

Factory version using @typedef

/** @typedef MyClass
 * @property email {string} The user's email
 * @property name {string} The user's name
 */

const proxies = new WeakSet();

/** Factory for MyClass proxy.
 * @returns {MyClass}
 */
export function MyClassFactory( target ) {
    const proxy = new Proxy( target, proxyHandler );
    proxies.add( proxy );
    return proxy;
}
const proxyHandler = {
    // lots of stuff here, but no getPrototypeOf trap
}

/** Whether it's a MyClass instance.
 * @returns {boolean}
 */
export function isInstanceOfMyClass( proxy ) {
    return proxies.has( proxy );
}

// ... later in another file ...
import MyClass from './MyClass.js';

/** @param {MyClass|OtherClass} the data
 */
getUserEmail( myInstance ) {
    if ( isInstanceOfMyClass( myInstance ) ) {  // and here is where we really confuse my future colleagues
        return myInstance.email;
    }
}

Why I don't like the workarounds

  1. The @typedef implementation above shows the kind of weirdness that happens if you need to use the functionality of instanceof with a dummy class. The entire implementation works fine, but reinventing the instanceof wheel sort of jumps off the future code maintenance cliff. Future junior programmers are not going to understand why the function isInstanceOfMyClass even exists, let alone go looking for it to use it until their code blows up.
  2. The original poster's workaround involves declaring properties on the dummy MyClass. I have implemented this workaround at this time, and it comes out to 90 lines of code/JSDocs for what should be 22 lines of JSDocs. However, none of any declared properties of MyClass would be actually reachable code, because the proxy is the true handler of every property. That means Istanbul code coverage fails for all the non-ugly implementations of the workaround. The only way to get it to work is to declare the properties inside the class constructor before you return the proxy. It's messy and, code-wise, pointless.
  3. If I make a d.ts file for this, it will be the only d.ts file in the whole (small) project, because I don't need any for anything else. I don't want to make exactly one d.ts file sitting inside an otherwise pure-JavaScript project solely to declare 22 properties this one time. Besides that, I would need to maintain the JSDoc @property statements anyway so that the JSDocs come out correctly. Documenting things twice is not good.
  4. This whole situation is obviously an oversight on your part. JSDoc has supported @property since its inception. It's not like JSDoc added some new feature and you haven't caught up yet. This should have been one of the first pieces of JSDoc-to-TypeScript support. Plus, you already support @property for @typedef, so there can't be that much extra code needed.

brodycj added a commit to brodycj/prettier that referenced this issue Aug 11, 2020
brodycj added a commit to brodycj/prettier that referenced this issue Aug 11, 2020
brodycj added a commit to brodycj/prettier that referenced this issue Aug 11, 2020
brodycj added a commit to brodycj/prettier that referenced this issue Jan 24, 2021
@jsg2021
Copy link

jsg2021 commented May 21, 2021

I've been trying to figure out how to accomplish this. Was there any resolution?

I just want to document that at runtime my class will have a "property" of "Type"... I could have sworn there was a way to document these properties.

@trusktr
Copy link
Contributor

trusktr commented Feb 15, 2022

Note that this syntax,

class DTO {
	/**
	 * @type {string | undefined}
	 */
	other;
}

introduces runtime behavior that will break code because the other field gets defined in constructor with an undefined value that can shadow anything a super() call may have put in place.

Here's an example of how it breaks. This is the plain JS code:

class Base {
  constructor() {
    this.init()
  }
}

class Subclass extends Base {
  init() {
    this.other = 123
  }
}

console.log(new Subclass().other) // logs "123"

Now we try to add types with JSDoc and it breaks:

class Base {
  constructor() {
    this.init()
  }
}

class Subclass extends Base {
  /** @type {number} */
  other;

  init() {
    this.other = 123
  }
}

console.log(new Subclass().other) // logs "undefined"

We need the equivalent of declare other: number in TS for plain JS / JSDoc.

I understand we can refactor code to make it work, but in some cases that may be a lot more work, especially in rather large plain JS projects.

@sandersn
Copy link
Member

A similar syntax works in constructors:

class C {
  constructor() {
    /** @type {number | undefined} */
    this.p
  }
}

@edmund-need2know
Copy link

edmund-need2know commented Nov 2, 2022

I THINK I might have found a way to solve some of this (at least as far as Webstorm intellisense is concerned) - excuse the shitty example

This does not work.

// base classes

class Wheel {
  rotate() {}
}

class Machine {
  /** @type {Wheel[]} */
  wheels;
}

// derived classes

class CarWheel extends Wheel {
  rotateOnRoad() {}
}

class Car extends Machine {
  constructor() {
    super();
    this.wheels.push(new CarWheel());
  }

  drive() {
    this.wheels.forEach((wheel) => {
      // this gives me an unresolved function warning as rotateOnRoad() does not exist on the base wheel class.
      wheel.rotateOnRoad();
    });
  }
}

But if I update the JSDOCS on my derived class with @typedef and @Property, I can redeclare the type of the inherited property without having to actually redeclare the property on the derived class (which would break the property at runtime).

/**
 * @typedef {Car} Car
 * @property {CarWheel[]} wheels
 */
class Car extends Machine {
  constructor() {
    super();
    this.wheels.push(new CarWheel());
  }

  drive() {
    this.wheels.forEach((wheel) => {
      // this now works as the type the iterate from this.wheels has been updated to {CarWheel}
      wheel.rotateOnRoad();
    });
  }
}

@barsdeveloper
Copy link

barsdeveloper commented Nov 3, 2022

But if I update the JSDOCS on my derived class with @typedef and @Property, I can redeclare the type of the inherited property without having to actually redeclare the property on the derived class (which would break the property at runtime).

/**
 * @typedef {Car} Car
 * @property {CarWheel[]} wheels
 */
class Car extends Machine {
  constructor() {
    super();
    this.wheels.push(new CarWheel());
  }

  drive() {
    this.wheels.forEach((wheel) => {
      // this now works as the type the iterate from this.wheels has been updated to {CarWheel}
      wheel.rotateOnRoad();
    });
  }
}

Causes a duplicate identifier error on VS Code

@clshortfuse
Copy link

clshortfuse commented Dec 8, 2022

I'm moving to supporting a more functional approach to my Web Components. When I'm working with a class, I can tap into .prototype and I don't need to specify anything:

export default class ExtendedFab extends Button {
  static { this.autoRegister('mdw-extended-fab'); }

  compose() {
    return super.compose().append(styles);
  }
}

// `.prototype.lowered` is actually useless in code. `idl()` calls defineProperty()
//  Typescript picks up .prototype calls and adds it as a class property.
ExtendedFab.prototype.lowered = ExtendedFab.idl('lowered', 'boolean');

Intellisense:
image


This is now the new syntax I want to support:

const ExtendedFab = Button.extend(styles);
ExtendedFab.autoRegister('mdw-extended-fab');
ExtendedFab.prototype.lowered = ExtendedFab.idl('lowered', 'boolean');
export default ExtendedFab;

It doesn't work because while Typescript does understand ExtendedFab to be class ExtendedFab, it also it to be a typeof the class returned by Button.extend.

image

It seems it's splitting there. I wonder if there's a way to strip that type, but I haven't found it.

image

@jimmywarting
Copy link
Contributor

Want support for @property too.

either:

/** @property {string} id */
class DTO {
  constructor(obj) {
    Object.assign(this, obj)
  }
}

or

class DTO {
  /** @property {string} id */
  
  constructor(obj) {
    Object.assign(this, obj)
  }
}

This one do not cut it... it introduce different runtime behavior

class DTO {
  /** @type {number} */
  id
}

@clshortfuse
Copy link

I've spent the last week or so writing small runtime cast functions. I've manage to get it to work with extended classes as well as mixins. I was holding off until it's done, but seeing @jimmywarting here 👋 , I might as well share my progress. I was able to get property casting working with this:

/**
 * @template T Class
 * @template {Object} P
 * @typedef { Omit<T, 'prototype'> &
 * {
 *  new (...args:ConstructorParameters<T>): InstanceType<T> & P
 * } & {
 *  prototype: InstanceType<T> & P
 * } } DefinedPropertiesOf
 */

/**
 * @template T Class
 * @template {Object} P
 * @param {T} Class
 * @param {P} props
 * @return {DefinedPropertiesOf<T,P>}
 */
const CastDefinedPropertiesOf = (Class, props) => Class;

With this, you can use what basically minifies down to a couple of bytes and TS will accept your casted input. With this, I was able to work out a functional syntax like this:

export default ExtendsMixin(Button, { lowered: 'boolean' })
  .compose(styles)
  .autoRegister('mdw-extended-fab');

With (lowered: 'boolean') being the properties in question. But it's not 100% yet, because I can only support one mixin without breaking it all. That's more related to lack of a polymorphic this as explained in #5863

I was able to create some other types with this "useless" function calling method, which are SubclassOf<S,C> and MixinOf<B,M>. I'm just happy I don't need to use the ES3 prototype syntax anymore (while I figure a way to support multiple mixins). It would be a bit easier TS had something like flow does for Class<T>, but I'm also working around it with this:

/**
 * @template T
 * @typedef {{
 *  new (...args:ConstructorParameters<T>): InstanceType<T>
 * } & {
 *   prototype: InstanceType<T>
 * } & {
 *  [P in keyof T]:T[P]
 * }} ClassOf
 */

/**
 * @template T
 * @param {T} Class
 * @return {ClassOf<T>}
 */
const CastClassOf = (Class) => Class;

And use @template {ClassOf<unknown>} T quite a bit.

@jimmywarting
Copy link
Contributor

jimmywarting commented Dec 16, 2022

damm, that's a lot for just simply using existing jsdoc @property
TypeScript should support this...

@clshortfuse
Copy link

@jimmywarting 100% agree. But I don't think we have a JSDoc champion here anymore. I've kinda given up on waiting on better JS support with typecheck. I'm tired of searching and finding my own comments from years/months ago 😞 . I've resigned to just having to build a solution myself, even if it needs a runtime cast.

I expanded your sample a bit with an improved typedef that shouldn't invoke TS warnings.

// TS Casts

/**
 * @template {abstract new (...args: any) => any} T Class
 * @template {Object} P
 * @typedef { Omit<T, 'prototype'> &
 * {
 *  new (...args:ConstructorParameters<T>): InstanceType<T> & P
 * } & {
 *  prototype: InstanceType<T> & P
 * } } DefinedPropertiesOf
 */

/**
 * @template {new (...args: any) => any} T Class
 * @template {Object} P
 * @param {T} Class
 * @param {P} props
 * @return {DefinedPropertiesOf<T,P>}
 */
const CastDefinedPropertiesOf = (Class, props) => Class;

// Code section

class DTO {
  constructor(obj) {
    Object.assign(this, obj)
  }
}

const DTO_SCHEMA = {id:''};

const DefinedDTO = CastDefinedPropertiesOf(DTO, DTO_SCHEMA);

// Export Cast instead of Class. Same runtime, parsed differently by TS.
export default DefinedDTO;

// Tests
const instance = new DefinedDTO();
instance.id = 'abc';
// @ts-expect-error Invalid boolean => String
instance.id = false;

You can also do:

DTO.prototype.id = /** @type {string} */ (undefined);

But, I've personally modified my CastDefinedProperties to call Object.defineProperties(DTO.prototype, for an observable pattern, so I don't actually lose much by doing it at runtime here.


If anybody wants to try to add it into the TS code, I believe it would here at https://github.com/microsoft/TypeScript/blob/4932c8788b9a0737d54a17a1d7c613b8f4daa6c2/src/compiler/checker.ts at lines 11346, with the function getLocalTypeParametersOfClassOrInterfaceOrTypeAlias or serializeAsClass.

The type checker is a bit daunting at 47,000 lines for one file, so I'm not really ready to jump into such a large file and start working on it. I rather do a typedef.

@what1s1ove
Copy link

what1s1ove commented Nov 17, 2023

Hey guys, I have the opposite question/suggestion.

Perhaps it is worth making it so that properties that are not described using @propety cannot be added to the class?

Now I can add any properties to the class using JSDoc (.js files) in conjunction with TypeScript, and I will not receive any errors.
image

But I would like similar behavior as in pure TypeScript:
image

Or perhaps you know of other ways to prohibit adding new properties without declaring them using JSDoc (.js files) in conjunction with TypeScript?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Domain: JavaScript The issue relates to JavaScript specifically Experience Enhancement Noncontroversial enhancements Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests