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

Support JSDoc @property #15715

Open
mjbvz opened this issue May 10, 2017 · 8 comments
Open

Support JSDoc @property #15715

mjbvz opened this issue May 10, 2017 · 8 comments
Labels
Committed The team has roadmapped this issue Domain: JavaScript The issue relates to JavaScript specifically Domain: JSDoc Relates to JSDoc parsing and type generation Suggestion An idea for TypeScript
Milestone

Comments

@mjbvz
Copy link
Contributor

mjbvz commented May 10, 2017

From @BrunnerLivio on May 8, 2017 9:1

  • VSCode Version: 1.12.1
  • OS Version: Ubuntu 16.04.1 LTS (Xenial Xerus)

Example code:
MyController.js

// @ts-check
class MyController {
    getWorld() {
        return this.world; // <--- Marked as error
    }
}
angular
    .module('myApp')
    .component('myComponent', {
        bindings: {
            'world': '<'
        },
        controller: MyController,
        template: `<div>Hello {{$ctrl.getWorld()}}</div>`
    });

The this.world is marked as error, as supposed to be. But it is actually pretty annoying, because in angular you need to use quite often those component bindings. I know there's the option // @ts-ignore, but that really pollutes the code and makes it less readable.

An extensions for this would be really useful.

I use this workaround at the moment

// @ts-check
class MyController {
    constructor() {
        this.world = this.world;
    }
    getWorld() {
        return this.world;
    }
}

[...]

I don't like this at all, but better than writing // @ts-ignore.

Is there any other option / workaround?

Copied from original issue: microsoft/vscode#26192

@mjbvz
Copy link
Contributor Author

mjbvz commented May 10, 2017

Yes I think that adding declarations in the constructor is the proper workaround for this issue. When TypeScript sees the MyController class, it cannot know that the class be used with a certain set of bindings.

@mhegazy / @chuckjaz Any other thoughts on this? Could the angular language service help out here?

@DanielRosenwasser
Copy link
Member

Any other thoughts on this? Could the angular language service help out here?

Not really, AngularJS (1.x) and Angular (2+) are very different. You could make a language service that specifically squelches errors and augments completions, but that's a bad a idea.

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented May 10, 2017

@BrunnerLivio are you using something like Babel, or just targeting ES6 runtimes? If you're using a compiler, you can declare your class properties as so and use the appropriate plugin:

class C {
    /** @type {string} */ world;
    
    foo() {
        this.world;
    }
}

@BrunnerLivio
Copy link

BrunnerLivio commented May 10, 2017

@DanielRosenwasser no, I'm not using Babel, I target a ES6 runtime. But thanks for the tip.

What about a more general solution (not AngularJS specific)? Maybe defining in a JSDoc comment above a Class, which properties get injected into a class.

For Example:

/*
* @inject {string} world
*/
class MyController {
  foo() {
    return this.world;
  }
}

Unfortunately @inject does not exist in JSDoc, but maybe you can use @property. But the definition of @Property is not fitting:

The @Property tag is a way to easily document a list of static properties of a class, namespace or other object.

Maybe other options?

@mhegazy
Copy link
Contributor

mhegazy commented May 10, 2017

@property would be the best way to go i would say.

@mjbvz
Copy link
Contributor Author

mjbvz commented Nov 9, 2018

See #28440 for another example of property tag usage:

/**
 * @description
 * A Person
 *
 * @class
 */
function Person(name) {
    /**
     * @description
     * Name of the person
     * @property {String}
     */
    this.name = name;

    /**
     * @description
     * Another Name of the person
     * @property {String}
     */
    this.anotherName = name;

    /**
     * @description
     * Another Name of the person
     * @property {String}
     * @name Person#AndEvenMoreNames
     */
}

@PavelPolyakov
Copy link

alright, I'd duplicate my case here:

// my-new-type.js

/**
 * MyNewType
 * @typedef {Object} MyNewType
 * @property {Factory~logFirst} logFirst
 * @property {Factory~logSecond} logSecond
 */

/**
 * MyNewType factory
 * @constructor
 * @param {number} first
 * @param {number} second
 * @returns MyNewType
 */
function Factory(first, second) {
  /**
   * logs first argument
   * @param {number} times
   */
  function logFirst(times) {
    for (let i = 0; i < times; i++) {
      console.log(first);
    }
  }

  /**
   * logs second argument
   * @param {number} times
   */
  function logSecond(times) {
    for (let i = 0; i < times; i++) {
      console.log(second);
    }
  }

  return {
    logFirst,
    logSecond
  };
}

module.exports = Factory;

The example above currently doesn't work, since it "does not see" child methods.

it's not only about @property, but about supporting jsdoc namepath as well.
http://usejsdoc.org/about-namepaths.html

Thanks for creating a great product.

Regards,

@weswigham weswigham added Domain: JavaScript The issue relates to JavaScript specifically and removed Salsa labels Nov 29, 2018
@sandersn sandersn removed this from the TypeScript 3.3 milestone Jan 16, 2019
@jimmywarting

This comment was marked as resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Committed The team has roadmapped this issue Domain: JavaScript The issue relates to JavaScript specifically Domain: JSDoc Relates to JSDoc parsing and type generation Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

9 participants