Skip to content

Fix compile on save issues#10615

Merged
zhengbli merged 7 commits into
microsoft:tsserverVS-WIPfrom
zhengbli:tsserverVS-WIP-fixUpdateExternalProj
Aug 31, 2016
Merged

Fix compile on save issues#10615
zhengbli merged 7 commits into
microsoft:tsserverVS-WIPfrom
zhengbli:tsserverVS-WIP-fixUpdateExternalProj

Conversation

@zhengbli
Copy link
Copy Markdown

Fixes issues:

  1. Avoid updating project compile on save options if it is not set in the request
  2. Only return ".ts" and ".tsx" files in the affected file list

Comment thread src/server/builder.ts
}
return Object.keys(fileNameSet);
const result: string[] = [];
for (const fileName in fileNameSet) {
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.

make a function shouldEmitFile:

function shouldEmitFile(scriptInfo: ScriptInfo) {
    return !scriptInfo.hasMixedContent;
}

then you can use it here and in calls to filter above?

Comment thread src/server/builder.ts Outdated

if (!fileInfo.isExternalModuleOrHasOnlyAmbientExternalModules()) {
return this.project.getFileNamesWithoutDefaultLib();
return map(filter(this.project.getScriptInfosWithoutDefaultLib(), shouldEmitFile), info => info.fileName);
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.

this chunk of code look awfully similar to the one above. Can you pull this into method in base class? Also can you replace pair map + filter with loop to avoid creating intermediate array?

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.

Actuall ygetScriptInfosWithoutDefaultLib is also implemented using filter but I don't think it is used anywhere else except for these two places. Can you have method Project.getAllEmittableFiles() that will internally fuse filter(is not default lib) + filter (should emit) + map (get file name) into one loop.

@vladima
Copy link
Copy Markdown
Contributor

vladima commented Aug 31, 2016

👍

@zhengbli zhengbli merged commit 95378aa into microsoft:tsserverVS-WIP Aug 31, 2016
@zhengbli zhengbli deleted the tsserverVS-WIP-fixUpdateExternalProj branch August 31, 2016 19:11
@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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants