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

Duplicated definition references found when Go to Definition #63895

Closed
ayqy opened this issue Nov 28, 2018 · 8 comments
Closed

Duplicated definition references found when Go to Definition #63895

ayqy opened this issue Nov 28, 2018 · 8 comments
Assignees
Labels
*as-designed Described behavior is as designed info-needed Issue requires more information from poster

Comments

@ayqy
Copy link

ayqy commented Nov 28, 2018

Current Behavior:

External extensions contributed definitions are duplicated with build-in extensions (eg. typescript related extensions), and there is no way for external extensions to know whether other definitions exist or not.

image

Expected Feature:

  • Provide an option for vscode.languages.registerDefinitionProvider to avoid such conflicts with build-in or other external DefinitionProviders.

  • Alternatively, provide an API to get build-in or all other definitions, just lift conflicts to external extensions.

Steps to Reproduce:

  1. Clone the test-def repo
  2. Open test-def folder with VS Code, and then Start Debugging
  3. Open test-def/src folder with [Extension Development Host], and open test-def/src/extensions.ts
  4. import * as test from './test/index'; Go to Definition of ./test/index
  5. Two definitions found

Further More:

The exactly same definitions will be filtered, so I tried to provide the same definition with build-in result, and failed as well:

public provideDefinition(
    document: vscode.TextDocument, position: vscode.Position, token: vscode.CancellationToken):
    Thenable<vscode.Location> {
        console.log('trigger GoDefinitionProvider at ' + Date.now());
    return new Promise((resolve) => {

        // ... Do some stuff to find definitions 

        let file = path.join(__dirname, '../src/test/index.ts');
        console.log('resolve some thing ' + file);

        vscode.workspace.openTextDocument(file).then(doc => {
            // default range
            // {startLineNumber: 13, startColumn: 1, endLineNumber: 22, endColumn: 29}
            let range = doc.validateRange(new vscode.Range(new vscode.Position(0, 0), new vscode.Position(99999, 99999)));
            
            //!!! Change startPosition.line to 12 will do the trick,
            // but there's no way to know other definitions exactly.
            // let range = doc.validateRange(new vscode.Range(new vscode.Position(12, 0), new vscode.Position(99999, 99999)));


            console.log(range);
            resolve(new vscode.Location(
                vscode.Uri.file(file),
                range
            ));
        });
    })
}
@jrieken jrieken added the info-needed Issue requires more information from poster label Nov 28, 2018
@jrieken
Copy link
Member

jrieken commented Nov 28, 2018

Yes, the design is to be cooperative and we only de-dupe items that are exactly the same. What I wonder is why your extension duplicates what types script already computes?

@ayqy
Copy link
Author

ayqy commented Nov 28, 2018

Thanks for your rapid response, and here is the situation:

We want to add supports for some custom import rules like React's Haste, such as:

// Definition at `project/any-path-to/A.js`
class A {}

// Any other reference at `project/any-other-path-to/index.js`, 
// which supported by our custom build system
import A from 'A'
// which supports ES Module as well
import A from 'full-relative-path-to/A.js'

Now, conflicts occurred

my extension duplicates what types script already computes

Honestly, another way to solve this is that we guess what typescript Provider can resolve and filter them out.

But it's not a reliable or beautiful solution as you know, if users install other Provider extensions or build-in extensions enhancement involved some day, this fragile strategy will be broken Immediately.

So a reliable way to control Provider results in such cases is needed.

@jrieken
Copy link
Member

jrieken commented Nov 28, 2018

it looks like you wanna be a language server plugin: https://github.com/Microsoft/TypeScript/wiki/Writing-a-Language-Service-Plugin

@ayqy
Copy link
Author

ayqy commented Nov 29, 2018

Good idea, and I will investigate this way then.

But from my point of view, it's a common situation: when we are keen to enhance Go to Definition functionality, we will see these duplications quickly. Although there are other options in typescript, if any other languages ?

P.S. Except for our custom build system, Webpack supports Resolve as well, and I think it's worth considering such general cases.

@jrieken
Copy link
Member

jrieken commented Nov 29, 2018

Yeah, there can remove duplicates but I don't wanna stretch the definition of a duplicate too much

@ayqy
Copy link
Author

ayqy commented Nov 29, 2018

Exactly, I know there are no strict duplication problems ( a kind of conflict only ), the reason why stress
'duplicates' here is just a trick for issue searching to someone facing the same situation, because it's not easy to find differences between internal Range(0,0, 28, 12) and Range(0,0, 0, 0) or Position(0, 0) in the definition result list.

So, just ignore 'duplicates' and focus on the conflict, and it's a feature request only :)

@ollyhayes
Copy link

We have a similar issue. We have a javascript lerna monorepo and in each package we have babel compiling /packages/package-name/src into /packages/package-name/lib. When navigating to definition accross packages, it takes you to the compiled version in /packages/package-name/lib instead of in `.../src'.

I wrote a DefinitionProvider which works and supplies the correct version, but the old .../lib definition is still given as well and you have to choose between them. Ideally I'd like to just be taken to the version in .../src without being asked, is this not possible?

@jrieken jrieken added the *as-designed Described behavior is as designed label Apr 30, 2019
@vscodebot
Copy link

vscodebot bot commented Apr 30, 2019

The described behavior is how it is expected to work. If you disagree, please explain what is expected and what is not in more detail. See also our issue reporting guidelines.

Happy Coding!

@vscodebot vscodebot bot closed this as completed Apr 30, 2019
@vscodebot vscodebot bot locked and limited conversation to collaborators Jun 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
*as-designed Described behavior is as designed info-needed Issue requires more information from poster
Projects
None yet
Development

No branches or pull requests

3 participants