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

go to definition doesn't work for certain module patterns #15004

Closed
barisusakli opened this issue Nov 4, 2016 · 10 comments
Closed

go to definition doesn't work for certain module patterns #15004

barisusakli opened this issue Nov 4, 2016 · 10 comments
Assignees
Labels
javascript JavaScript support issues upstream Issue identified as 'upstream' component related (exists outside of VS Code)

Comments

@barisusakli
Copy link

barisusakli commented Nov 4, 2016

  • VSCode Version: 1.7.1
  • OS Version: windows 10

Steps to Reproduce:

  1. Write a module in a certain way
  2. Import the module and try to go to definition on a function.

Sample module A

exports.myFunction = function () {
    
};

Sample module B

var myModule =  {};
myModule.myFunction = function() {

};
exports = myModule;
//app code
var myModule = require('./myModule');
myModule.myFunction(); //go to definition doesn't work if you use sample module B, it works with module A

Full repo that reproduces issue https://github.com/barisusakli/vs-code-intellisense-test.

It seems like it only works if you set properties directly on exports.

@klinquist
Copy link

klinquist commented Nov 6, 2016

+1. I typically use:

module.exports = function(baseUrl){
  this.apiCall = function(url, cb) {
     let fullUrl = baseUrl + url;
  };

  return this;
}

'Go to definition' does not work when I require the file containing this function.

@waderyan
Copy link

waderyan commented Nov 7, 2016

@bowdenk7 would you call this an expando pattern limitation?

@barisusakli barisusakli added javascript JavaScript support issues code-navigation labels Nov 7, 2016
@bowdenk7
Copy link

bowdenk7 commented Nov 7, 2016

Yes and no... the way we do commonJS module resolution is by looking explicitly for module.exports or exports and then resolving modules using those objects. When those objects are aliased, currently we don't do any analysis to find and use the aliases. Is there a reason you prefer this approach as opposed to using exports or module.exports?

The second case from @klinquist looks like a bug to me.

@barisusakli
Copy link
Author

barisusakli commented Nov 8, 2016

We have a lot of existing code that uses module B or a variation of it, we would have to rewrite all the code to directly set on exports.

Not sure how sublime text 3 handles this but it can understand all of the different module patterns, I am guessing it is just doing some kind of text search. I stumbled upon https://marketplace.visualstudio.com/items?itemName=jrieken.fuzzy-definitions but it doesn't work as well as ST3 go to definition.

The below also works btw,

var myModule =  {
   myFunction: function () {}
};
exports = myModule;

So an alias kind of works but the alias object has to be created as an object literal.

@waderyan
Copy link

@klinquist can you open a separate issue in the TypeScript repo?

@abgivant
Copy link

abgivant commented Nov 15, 2016

I use a similar module pattern in an application I'm working on, and the lack of IntelliSense for those has been an issue for me as well.

Mine follow this basic pattern:

module.exports = function(x, y, z) {
  var exports = {};
  exports.method = function() {
    // Something that might use x, y, or z
  };
  return exports;
}

Would it be possible to do something like include an option in the jsconfig.json file to specify a property name or list of names to track when determining module exports?

Maybe something like this?

{
  "moduleOptions": {
    "exportProperty": ["exports", "this"]
  }
}

@waderyan waderyan added the upstream Issue identified as 'upstream' component related (exists outside of VS Code) label Nov 17, 2016
@mjbvz mjbvz self-assigned this Jan 26, 2017
@mjbvz
Copy link
Collaborator

mjbvz commented Jan 27, 2017

Closing as a duplicate of microsoft/TypeScript#10868 on the TypeScript team.

We cannot fix this on the VSCode side, but are coordinating with the TypeScript team to prioritize IntelliSense for properties that are dynamically added to objects.

@mjbvz mjbvz closed this as completed Jan 27, 2017
@barisusakli
Copy link
Author

@mjbvz thanks for the update. I will keep an eye on the other issue.

@pciazynski
Copy link

It's very annoying, I'll hope that TypeScript team fix this quickly :)

@PRossetti
Copy link

PRossetti commented Sep 7, 2017

@barisusakli Fuzzy Definitions made the trick for me, for now at least. Sadly it still doesn't work if I want to search a function definition from an HTML file. For example,
<button ng-click="myController.someFunction()">Bla bla</button> I'll get no results by trying to go to someFunction definition.
Is wierd for me that it only has 2514 downloads! It should have a lot more, it's the only thing that have worked for me and I tried a lot! Unless there is another better working extension that I don't know about... Is there?

Hope someone is working on this !
Thank you very much!

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
javascript JavaScript support issues upstream Issue identified as 'upstream' component related (exists outside of VS Code)
Projects
None yet
Development

No branches or pull requests

8 participants