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

JavaScript IntelliSense Should Recognize DefineProperty #26082

Closed
mjbvz opened this issue Jul 31, 2018 · 5 comments · Fixed by #27208
Closed

JavaScript IntelliSense Should Recognize DefineProperty #26082

mjbvz opened this issue Jul 31, 2018 · 5 comments · Fixed by #27208
Labels
Duplicate An existing issue was already created

Comments

@mjbvz
Copy link
Contributor

mjbvz commented Jul 31, 2018

From @johnnykahalawai on July 30, 2018 19:19

Issue Type: Feature Request

Hi -

Often when working with JavaScript an object i.e. class is extended with Object.defineProperty

Reference: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/defineProperty

Unfortunately, Visual Studio's IntelliSense cannot make this connection so the GoTo Definition feature does not work.

FWIW, JetBrain's products e.g. WebStorm and IntelliJ does support this feature.

Thanks in advance,

Johnny

VS Code version: Code 1.25.1 (1dfc5e557209371715f655691b1235b6b26a06be, 2018-07-11T15:33:29.235Z)
OS version: Darwin x64 17.7.0

Copied from original issue: microsoft/vscode#55390

@mjbvz mjbvz self-assigned this Jul 31, 2018
@mjbvz mjbvz added the Domain: JavaScript The issue relates to JavaScript specifically label Jul 31, 2018
@mjbvz
Copy link
Contributor Author

mjbvz commented Jul 31, 2018

Likely duplicate of Duplicate of #6651

@mjbvz mjbvz removed the Domain: JavaScript The issue relates to JavaScript specifically label Jul 31, 2018
@mjbvz mjbvz removed their assignment Jul 31, 2018
@mjbvz mjbvz added the VS Code Tracked There is a VS Code equivalent to this issue label Jul 31, 2018
@mhegazy mhegazy added Duplicate An existing issue was already created and removed VS Code Tracked There is a VS Code equivalent to this issue labels Jul 31, 2018
@johnnykahalawai
Copy link

johnnykahalawai commented Jul 31, 2018

@mjbvz --

Not exactly. More like this:

`function Person(name) {
this.name = name;
}

    Person.prototype.describe = function () {
      return "Person called " + this.name;
    };

    Object.defineProperty(Person.prototype, "firstInitial", {
        writable: false,
        value: function () {
          return this.name && this.name.substr(0, 1);
        }
    });

    var p = new Person("Johnny");

`
screen shot 2018-07-31 at 10 32 55 am

@mhegazy
Copy link
Contributor

mhegazy commented Jul 31, 2018

@johnnykahalawai they are the same issue. the type system does not recognize calls to Object.defineProperty as declarations of new properties regardless of their target. #6651 tracks adding that support.

@johnnykahalawai
Copy link

Hi @mhegazy -

I see. One man's opinion here but without defineProperty support salsa is missing the cilantro.

Aloha,

Johnny

@AbdelrahmanHafez
Copy link

Another example would be:

// add.js

function add (x, y) {
  return x + y;
}

module.exports = add;

Example 1 (adding getter directly to the object):

// index.js

const o = {
  get add () {
    return require('./add');
  }
};

const sum = o.add(2, 3); // 5

In example 1, Intellisense recognizes the add function from add.js, and knows its parameters, etc.

screenshot_7

Example 2 (adding getter via defineProperty):

// index.js

const o = {};

Object.defineProperty(o, 'add', {
  get: function () {
    return require('./add');
  }
});

const sum = o.add(2, 3); // 5

In example 2, Intellisense fails to recognize the function and mistakenly expects the function to return a promise.

screenshot_882

I am working on a utility that would allow the developers to have cleaner, aesthetic imports, where this feature is necessary to be useful for the developers who will use it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants