microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
101.26k stars 12.52k forks source link

A way to specify class properties using JSDoc #26811

Open arendjr opened 6 years ago

arendjr commented 6 years ago

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:

RyanCavanaugh commented 6 years ago

@sandersn can this be accomplished with declaration merging today?

sandersn commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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.

zzswang commented 4 years ago

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 commented 4 years ago

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

ExE-Boss commented 4 years ago

Originally posted by @zzswang in https://github.com/microsoft/TypeScript/issues/26811#issuecomment-655872427

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 commented 4 years ago

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 commented 4 years ago

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.
jsg2021 commented 3 years ago

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 commented 2 years ago

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 commented 2 years ago

A similar syntax works in constructors:

class C {
  constructor() {
    /** @type {number | undefined} */
    this.p
  }
}
edmund-need2know commented 2 years ago

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 commented 2 years ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 https://github.com/microsoft/TypeScript/issues/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 commented 1 year ago

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

clshortfuse commented 1 year ago

@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 commented 1 year ago

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?