Skip to content

Fix bug: Get mapped location of definition for findAllReferencesFull#27113

Merged
1 commit merged intomasterfrom
findAllRefsFull
Sep 17, 2018
Merged

Fix bug: Get mapped location of definition for findAllReferencesFull#27113
1 commit merged intomasterfrom
findAllRefsFull

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Sep 15, 2018

Fixes #26175

@ghost ghost requested review from amcasey and sheetalkamat September 17, 2018 17:57
@amcasey
Copy link
Copy Markdown
Member

amcasey commented Sep 17, 2018

I think @uniqueiniquity was having some trouble in related scenarios.

@uniqueiniquity
Copy link
Copy Markdown
Contributor

@amcasey I did, but I'm not sure whether I had my project configured correctly or not.

initialLocation: TLocation,
projectService: ProjectService,
cb: (where: ProjectAndLocation<TLocation>, getMappedLocation: (project: Project, location: sourcemaps.SourceMappableLocation) => boolean) => void,
cb: CombineProjectOutputCallback<TLocation>,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I find it confusing that some callers pass a callback with a different goal (tryAddToTodo) and no intention of returning. Maybe the return type should also be a type parameter?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean no intention of using the return value of getMappedLocation? Though every caller uses the return type of at least to test whether there was something added. It's true that they use it like a boolean but I don't think there's a type-safe way to have the return value be a type parameter that's either boolean or sourcemaps.SourceMappableLocation | undefined.

@ghost ghost merged commit bfc0093 into master Sep 17, 2018
@ghost ghost deleted the findAllRefsFull branch September 17, 2018 21:14
@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 21, 2025
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants