Skip to content

Deduplicate unresolvedImports#17248

Merged
3 commits merged into
masterfrom
unresolvedImports
Aug 8, 2017
Merged

Deduplicate unresolvedImports#17248
3 commits merged into
masterfrom
unresolvedImports

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Jul 17, 2017

Looking through some typings installer logs, I noticed the same types modules repeated many times in unresolvedImports. Since they're already sorted, it's easy to deduplicate them.

@ghost ghost requested a review from sheetalkamat July 17, 2017 20:37
Comment thread src/compiler/core.ts Outdated
}

export function filterMutate<T>(array: T[], f: (x: T) => boolean): void {
export function filterMutate<T>(array: T[], f: (x: T, i: number) => boolean): void {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If you align more closely with Array.prototype.filter and use (x: T, i: number, array: T[]) => boolean, then you can remove the closure in toDeduplicatedSortedArray by extracting it into a reusable function:

function isNonDuplicate<T>(value: T, index: number, array: T[]) {
  return i === 0 || value !== array[index - 1]; 
} 

Comment thread src/server/utilities.ts Outdated

export function toDeduplicatedSortedArray(arr: string[]): SortedArray<string> {
arr.sort();
filterMutate(arr, (em, i) => em !== arr[i - 1]);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

check that i is non-zero or you deoptimize access to the array.

@ghost ghost merged commit 5141ce7 into master Aug 8, 2017
@ghost ghost deleted the unresolvedImports branch August 8, 2017 18:02
@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.

2 participants