microsoft / TypeScript

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

GTD not working between ES5-style classes #31315

Open amcasey opened 5 years ago

amcasey commented 5 years ago

Reported in VS, repros in VS Code.

https://developercommunity.visualstudio.com/content/problem/556364/javascript-go-to-definition-or-f12-doesnt-work-wit.html

(function () {

    my.namepsace.myclass = function () {
        something: function something() {

        };
    };

    my.namespace.mystaticmethod = function () {
        return false;
    };

})();

(function () {

    another.namespace.class = function () {
        init: function init() {
            var myclass = new my.namepsace.myclass(); // GTD on myclass
            var result = my.namespace.mystaticmethod(); // GTD on mystaticmethod
        }
    };

})();

GTD does not work on the references to myclass and mystaticmethod.

orta commented 5 years ago

OK , looking at this, I'm not sure what to do with my.namespace here ( I assume they are somehow exposed to the global namespacesomehow. It looks like our ability to do GTD doesn't work when the JS code is embedded inside the IIFE

When I took it our of IIFEs then it started to work fine. Maybe the extension of x with the net properties is dependent on it's parent being the source-file and it could be extended to include IIFEs?

Sample which does GTD

var x = {};

// (function () {

    x.myclass = function () {
        something: function something() {

        };
    };

    x.mystaticmethod = function () {
        return false;
    };

// })();

// (function () {

    x.class = function () {
        init: function init() {
            var myclass = new x.myclass(); // GTD on myclass
            var result = x.mystaticmethod(); // GTD on mystaticmethod
        }
    };
x
// })();
orta commented 5 years ago

So this relates to how we extend an object in JS

var yourNamespace = {
  a: () => {},
  b: () => {}
};

yourNamespace.c = () => { }

(function () {
  yourNamespace.d = () => { }
})()

yourNamespace. // a & b only

@RyanCavanaugh / @sandersn - do we want to extend an object's type if it has a property written to it? We don't in TypeScript and that's purposeful to my knowledge, but maybe it doesn't have to have the same behavior in JS?

Sidenote: this bug comes from someone not using TS Server and expecting this behavior - so it's probably more of a feature request.

automaton82 commented 5 years ago

Sidenote: this bug comes from someone not using TS Server and expecting this behavior - so it's probably more of a feature request.

I was the one who reported this issue through Visual Studio 2019. And yes, I reported it because 2019 no longer gives the option to turn off the 'Language Service.'

So what else am I supposed to do here. It's technically vanilla JavaScript, regardless of TS or whatever engine doing the parsing.

orta commented 5 years ago

Ah yeah, calling it a feature request and not a bug wasn't a value judgement. 👍

It does mean that we can't build it into TypeScript 3.6 now that it's in feature freeze though.

automaton82 commented 5 years ago

Ah yeah, calling it a feature request and not a bug wasn't a value judgement. 👍

It does mean that we can't build it into TypeScript 3.6 now that it's in feature freeze though.

Yea that's fair. I was more just venting my frustration that VS pushes these features / bugs onto TypeScript because it so happens that's what they solely use now for all JS parsing (TS or not).

I doubt this kind of parsing is related to TS at all. But it just comes here because of changes in VS2019 that resulted in a loss of functionality.

orta commented 5 years ago

In this case moving it to the TS project is right - this is actually TypeScript's domain.

TypeScript handles the Language Service parts of the visual studio editors now and it can handle JavaScript too, this is why the issue landed here.

Part of what makes the JS side tricky is that we need to figure out where do we change the behavior of the editor integration specifically to allow for the more flexible nature of JS. This issue is a great example of where that kind of context is important.