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

Navigate-to does not pick up any identifiers in class expression #4481

Closed
DanielRosenwasser opened this issue Aug 26, 2015 · 11 comments
Closed
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@DanielRosenwasser
Copy link
Member

var ffffffffffff = class Pnsndltn {
    public raggafragga;
}

Try searching for any of the identifiers in the above with navigate-to.

Expected: All of them are found.
Actual: Only ffffffffffff is found.

Probable cause: computeNamedDeclarations doesn't account for ClassExpression kinds.

@DanielRosenwasser
Copy link
Member Author

Should this work for function expressions too? @mhegazy @CyrusNajmabadi

@mhegazy
Copy link
Contributor

mhegazy commented Oct 3, 2015

We chose to not include functions in the past, just to avoid polluting the list. I think class expressions are different enough; I woul not mind trying the experience with function expressions as well.

@mhegazy mhegazy modified the milestones: TypeScript 2.0, TypeScript 1.7 Oct 6, 2015
@mhegazy mhegazy modified the milestones: TypeScript 1.8, TypeScript 2.0 Dec 1, 2015
@mhegazy mhegazy assigned mhegazy and unassigned DanielRosenwasser Apr 25, 2016
@mhegazy mhegazy assigned ghost and unassigned mhegazy May 12, 2016
@ghost ghost mentioned this issue May 13, 2016
@ghost ghost closed this as completed in #8595 May 17, 2016
@mhegazy mhegazy added the Fixed A PR has been merged for this issue label May 17, 2016
@scarlac
Copy link

scarlac commented Oct 10, 2017

@mhegazy While this was fixed for classes, I have an existing code base with a lot of modules written in the style of:

module.exports = function() {
  const methods = {};
  methods.myFirstFunction = function() { /* ... */ };
  methods.mySecondFunction = function() { /* ... */ };
  return methods;
}

What is counter-intuitive (by VSCode's own standard) is that it'll show these functions through "Goto Symbol in File" while "Goto Symbol in Workspace" will not show it. This discrepancy is very confusing and does not make sense. Anecdotally, Sublime does this and I have no issue using it - I never scroll through the list, just like I don't scroll through the list when using the command palette. I understand the argument that it may pollute, although I cannot confirm that I agree as I haven't seen it enabled, but the UX experience is really bad. It comes off as a bug.

Is there any way we could improve the experience or have it as a configuration setting?

@CyrusNajmabadi
Copy link
Contributor

@scarlac That looks like an entirely different issue. I would recommend creating a new bug to track this.

@scarlac
Copy link

scarlac commented Oct 11, 2017

@CyrusNajmabadi, I followed issues for that to this ticket. The others were closed with a reference to this one. Should I still create a new one?

@mhegazy
Copy link
Contributor

mhegazy commented Oct 11, 2017

The underlying issue is that the type system does not understand methods.myFirstFunction = function() { /* ... */ }; as a declaration of a new property myFirstFunction on methods.

see #7632, #9474, #10868 and #15868

@scarlac
Copy link

scarlac commented Oct 11, 2017

@mhegazy That sounds odd as it works when using "Go to Symbol in File". Was a specific workaround implemented to make it work there and not in the workspace?

@CyrusNajmabadi
Copy link
Contributor

Is "go to symbol in file" using a different engine (perhaps just a textual one), rather than using the TS language server?

@ghost
Copy link

ghost commented Oct 12, 2017

@scarlac It looks like "Go to Symbol in Workspace" requires you to start typing the name.
shot

The navigation features don't use the type system, they use custom code in navigationBar.ts and navigateTo.ts, so you can navigate to things that the type system doesn't understand.

@scarlac
Copy link

scarlac commented Oct 12, 2017

@andy-ms I appreciate that explanation. As a user I just need it to work. The only reasonable reason I've heard is that it's too many results for it to be practical, but Sublime does it and it works very well so I can't entirely agree with that argument. I wouldn't mind posting a separate issue but it seems I'm not the first to point this out - the request just keeps being tacked on to other issues without being directly addressed.

Right now, Sublime gives me better practical code navigation even though VSCode has much deeper code understanding. Thus, VSCode should theoretically be better at navigation and I think it's just a matter of pushing the technical boundaries a bit :)

@ghost
Copy link

ghost commented Oct 13, 2017

You might want to make an issue on VSCode then? The TypeScript language service is the same for Sublime and VSCode.

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

4 participants