Skip to content

LanguageServices.getDocumentHighlights does not find references within JSX #4175

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

Closed
adidahiya opened this issue Aug 6, 2015 · 13 comments
Closed
Assignees
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@adidahiya
Copy link
Contributor

Using v1.6.0-dev.20150805.

For the following code, LanguageServices.getDocumentHighlights reports only 1 highlight span for onChange, where it should really report 2:

test.tsx

import * as React from "react";

export class Foo extends React.Component<{}, {}> {
    public render() {
        return (
            <div>
                <input onChange={this.onChange.bind(this)} />
            </div>
        );
    }

    onChange() {
        console.log("changed");
    }
}

You can reproduce this error with the sample code from #3688 (copied below):

var ts = require('typescript');
var fs = require('fs');

var source = fs.readFileSync('test.tsx').toString();
var sourceFile = ts.createSourceFile('test.tsx', source, 1, false);

var host = {
  getCompilationSettings: function() {
      return {
          noResolve: true,
          target: ts.ScriptTarget.ES5
      };
  },
  getCurrentDirectory: function() {
    return '';
  },
  getDefaultLibFileName: function() {
    return 'lib.d.ts';
  },
  getScriptFileNames: function() {
    return ['test.tsx'];
  },
  getScriptIsOpen: function() {
    return true;
  },
  getScriptVersion: function() {
    return '1';
  },
  getScriptSnapshot: function() {
    return ts.ScriptSnapshot.fromString(source);
  }
};

var documentRegistry = ts.createDocumentRegistry();
var languageServices = ts.createLanguageService(host, documentRegistry);

var onChangeFn = sourceFile.statements[1].members[1];
var highlights = languageServices.getDocumentHighlights('test.tsx', onChangeFn.name.pos, ['test.tsx']);
console.log(highlights[0].highlightSpans.length);
@DanielRosenwasser DanielRosenwasser added the Bug A bug in TypeScript label Aug 6, 2015
@DanielRosenwasser
Copy link
Member

I assume FindAllRefs and GoToDefinition also doesn't work. Will need to test this.

@RyanCavanaugh
Copy link
Member

This is odd -- it's working in Sublime Text

image

@adidahiya
Copy link
Contributor Author

Note: I edited test.tsx above to remove the private modifier, which made the repro code incorrect.

In trying to debug this, I found that ts.getTouchingWord returns the node for the whole onChange method instead of just the node for the onChange identifier. This in turn causes getDocumentHighlights to return undefined.

@ashwinr
Copy link

ashwinr commented Aug 13, 2015

presume this also happens for variable references from within JSX.

@RyanCavanaugh
Copy link
Member

The updated sample still works fine in Sublime Text. The posted code using the LS directly gets undefined for highlights - ?

@RyanCavanaugh RyanCavanaugh added the Needs More Info The issue still hasn't been fully clarified label Aug 20, 2015
@jkillian
Copy link

@RyanCavanaugh Is there any trick to finding references in a .tsx file in Sublime? For some reason I can't get it to work.

Also, in your above screenshot, the first onChange shouldn't be marked as a reference, should it?
<input *onChange*={this.onChange.bind(this)} />
How does the Sublime plugin find references?

@RyanCavanaugh
Copy link
Member

You'll need to manually set the Syntax for .tsx files to TypeScript. Then all the regular TS features should work.

The Sublime plugin finds references the same way the VS plugin does -- using the compiler language service.

@adidahiya
Copy link
Contributor Author

Maybe we're using the wrong feature in the language service to get references? Is getDocumentHighlights not the right approach?

I think @jkillian is right -- the onChange attribute name in the <input ...> JSX element in your screenshot should not be counted as a reference to the class's onChange method.

@RyanCavanaugh
Copy link
Member

the onChange attribute name in the <input ...> JSX element in your screenshot should not be counted as a reference to the class's onChange method.

This is correct, and the behavior is also correct. In the "Find References" pane, it's string-based underlining. Here, I did "Find References" on the attribute name and found two references (the usage in the element, and the definition on the .d.ts file). It reports two references even though there are four things underlined.

image

Conversely, if I Find References on the class method, it reports the correct three references (modified the code slightly from above to show this more clearly). Again, the underlining is string-based and doesn't reflect the actual references.

image

@jkillian
Copy link

The code in the OP doesn't work because onChangeFn.name.pos gives the position at the beginning of the line and not where the text 'onChange' starts. However, I'm having the same issue with other examples.

@RyanCavanaugh Care to see how this code does in Sublime? Specifically onClick and bar. For some reason I still can't get 'FindReferences' to work on .tsx files: I can run the 'Find References' command from the command palette, but then nothing happens.

import * as React from "react";

export class FooComponent extends React.Component<{}, {}}> {
    public render() {
        return (
            <div onClick={() => this.onClick()}>
                {this.state.bar.map((s) => <span>{s}</span>)}
                <BazComponent someProp={123} anotherProp={456} />
            </div>
        );
    }

    private onClick() {
        console.info("click");
    }
}

export function buildFooComponent(): JSX.Element {
    let bar: string = "test";
    return <FooComponent fooProp={ bar } />;
}

@RyanCavanaugh
Copy link
Member

@jkillian logged #4404

@RyanCavanaugh
Copy link
Member

#4406 should fix a broad class of issues here. Anyone care to try the latest master ?

@jkillian
Copy link

@RyanCavanaugh this appears to have fixed the bug mentioned in my previous comment, thanks!

@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Aug 26, 2015
@mhegazy mhegazy closed this as completed Aug 26, 2015
@RyanCavanaugh RyanCavanaugh removed the Needs More Info The issue still hasn't been fully clarified label Aug 26, 2015
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
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

6 participants