Skip to content

Remove createFileMap#16810

Merged
3 commits merged into
masterfrom
createFileMap
Jul 10, 2017
Merged

Remove createFileMap#16810
3 commits merged into
masterfrom
createFileMap

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Jun 28, 2017

After #16724, createFileMap is now only used in program.ts.
We had publicly exposed the FileMap type before, but we weren't exposing any values of that type. So we can make it internal to program.ts. Also removed all unused methods and renamed others to resemble those in Map.

@ghost ghost requested review from amcasey and sandersn June 28, 2017 20:27
Comment thread src/compiler/program.ts Outdated
has(fileName: Path): boolean;
}

function createFileMap<T>(keyMapper: (key: string) => string): FileMap<T> {
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.

Should keyMapper take a Path?

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.

I had a closer look at all the uses and couldn't answer this question. They were either using keyMapper redundantly (meaning, they were canonicalizing paths multiple times) or using it to convert things to lower case. It's simpler to just remove createFileMap entirely.

@ghost ghost changed the title Make createFileMap an internal detail of program.ts Remove createFileMap Jun 28, 2017
@ghost ghost force-pushed the createFileMap branch from 38feaf9 to 738b3bc Compare June 28, 2017 22:02
@ghost ghost merged commit 8c3f5e2 into master Jul 10, 2017
@ghost ghost deleted the createFileMap branch July 10, 2017 18:24
gcnew added a commit to gcnew/TypeScript that referenced this pull request Jul 10, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
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