Skip to content
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

Exclude js files in non-configured projects compile-on-save emitting #12118

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 8 additions & 15 deletions src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1690,18 +1690,13 @@ namespace ts {
const emitFilePath = toPath(emitFileName, currentDirectory, getCanonicalFileName);
// Report error if the output overwrites input file
if (filesByName.contains(emitFilePath)) {
if (options.noEmitOverwritenFiles && !options.out && !options.outDir && !options.outFile) {
blockEmittingOfFile(emitFileName);
}
else {
let chain: DiagnosticMessageChain;
if (!options.configFilePath) {
// The program is from either an inferred project or an external project
chain = chainDiagnosticMessages(/*details*/ undefined, Diagnostics.Adding_a_tsconfig_json_file_will_help_organize_projects_that_contain_both_TypeScript_and_JavaScript_files_Learn_more_at_https_Colon_Slash_Slashaka_ms_Slashtsconfig);
}
chain = chainDiagnosticMessages(chain, Diagnostics.Cannot_write_file_0_because_it_would_overwrite_input_file, emitFileName);
blockEmittingOfFile(emitFileName, createCompilerDiagnosticFromMessageChain(chain));
let chain: DiagnosticMessageChain;
if (!options.configFilePath) {
// The program is from either an inferred project or an external project
chain = chainDiagnosticMessages(/*details*/ undefined, Diagnostics.Adding_a_tsconfig_json_file_will_help_organize_projects_that_contain_both_TypeScript_and_JavaScript_files_Learn_more_at_https_Colon_Slash_Slashaka_ms_Slashtsconfig);
}
chain = chainDiagnosticMessages(chain, Diagnostics.Cannot_write_file_0_because_it_would_overwrite_input_file, emitFileName);
blockEmittingOfFile(emitFileName, createCompilerDiagnosticFromMessageChain(chain));
}

// Report error if multiple files write into same file
Expand All @@ -1716,11 +1711,9 @@ namespace ts {
}
}

function blockEmittingOfFile(emitFileName: string, diag?: Diagnostic) {
function blockEmittingOfFile(emitFileName: string, diag: Diagnostic) {
hasEmitBlockingDiagnostics.set(toPath(emitFileName, currentDirectory, getCanonicalFileName), true);
if (diag) {
programDiagnostics.add(diag);
}
programDiagnostics.add(diag);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3060,7 +3060,7 @@ namespace ts {
moduleResolution?: ModuleResolutionKind;
newLine?: NewLineKind;
noEmit?: boolean;
/*@internal*/noEmitOverwritenFiles?: boolean;
/*@internal*/noEmitForJsFiles?: boolean;
noEmitHelpers?: boolean;
noEmitOnError?: boolean;
noErrorTruncation?: boolean;
Expand Down
58 changes: 31 additions & 27 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ namespace ts {
export function nodePosToString(node: Node): string {
const file = getSourceFileOfNode(node);
const loc = getLineAndCharacterOfPosition(file, node.pos);
return `${ file.fileName }(${ loc.line + 1 },${ loc.character + 1 })`;
return `${file.fileName}(${loc.line + 1},${loc.character + 1})`;
}

export function getStartPosOfNode(node: Node): number {
Expand Down Expand Up @@ -439,7 +439,7 @@ namespace ts {
}

export function isBlockScope(node: Node, parentNode: Node) {
switch (node.kind) {
switch (node.kind) {
case SyntaxKind.SourceFile:
case SyntaxKind.CaseBlock:
case SyntaxKind.CatchClause:
Expand Down Expand Up @@ -624,9 +624,9 @@ namespace ts {

export function getJsDocCommentsFromText(node: Node, text: string) {
const commentRanges = (node.kind === SyntaxKind.Parameter ||
node.kind === SyntaxKind.TypeParameter ||
node.kind === SyntaxKind.FunctionExpression ||
node.kind === SyntaxKind.ArrowFunction) ?
node.kind === SyntaxKind.TypeParameter ||
node.kind === SyntaxKind.FunctionExpression ||
node.kind === SyntaxKind.ArrowFunction) ?
concatenate(getTrailingCommentRanges(text, node.pos), getLeadingCommentRanges(text, node.pos)) :
getLeadingCommentRangesOfNodeFromText(node, text);
return filter(commentRanges, isJsDocComment);
Expand Down Expand Up @@ -891,7 +891,7 @@ namespace ts {
export function isObjectLiteralOrClassExpressionMethod(node: Node): node is MethodDeclaration {
return node.kind === SyntaxKind.MethodDeclaration &&
(node.parent.kind === SyntaxKind.ObjectLiteralExpression ||
node.parent.kind === SyntaxKind.ClassExpression);
node.parent.kind === SyntaxKind.ClassExpression);
}

export function isIdentifierTypePredicate(predicate: TypePredicate): predicate is IdentifierTypePredicate {
Expand Down Expand Up @@ -1113,8 +1113,8 @@ namespace ts {
// if the parameter's parent has a body and its grandparent is a class declaration, this is a valid target;
return (<FunctionLikeDeclaration>node.parent).body !== undefined
&& (node.parent.kind === SyntaxKind.Constructor
|| node.parent.kind === SyntaxKind.MethodDeclaration
|| node.parent.kind === SyntaxKind.SetAccessor)
|| node.parent.kind === SyntaxKind.MethodDeclaration
|| node.parent.kind === SyntaxKind.SetAccessor)
&& node.parent.parent.kind === SyntaxKind.ClassDeclaration;
}

Expand Down Expand Up @@ -1461,7 +1461,7 @@ namespace ts {
}, tags => tags);
}

function getJSDocs<T>(node: Node, checkParentVariableStatement: boolean, getDocs: (docs: JSDoc[]) => T[], getTags: (tags: JSDocTag[]) => T[]): T[] {
function getJSDocs<T>(node: Node, checkParentVariableStatement: boolean, getDocs: (docs: JSDoc[]) => T[], getTags: (tags: JSDocTag[]) => T[]): T[] {
// TODO: Get rid of getJsDocComments and friends (note the lowercase 's' in Js)
// TODO: A lot of this work should be cached, maybe. I guess it's only used in services right now...
let result: T[] = undefined;
Expand All @@ -1482,8 +1482,8 @@ namespace ts {

const variableStatementNode =
isInitializerOfVariableDeclarationInStatement ? node.parent.parent.parent :
isVariableOfVariableDeclarationStatement ? node.parent.parent :
undefined;
isVariableOfVariableDeclarationStatement ? node.parent.parent :
undefined;
if (variableStatementNode) {
result = append(result, getJSDocs(variableStatementNode, checkParentVariableStatement, getDocs, getTags));
}
Expand Down Expand Up @@ -1648,7 +1648,7 @@ namespace ts {
if ((<ShorthandPropertyAssignment>parent).name !== node) {
return AssignmentKind.None;
}
// Fall through
// Fall through
case SyntaxKind.PropertyAssignment:
node = parent.parent;
break;
Expand Down Expand Up @@ -2550,14 +2550,18 @@ namespace ts {
if (options.outFile || options.out) {
const moduleKind = getEmitModuleKind(options);
const moduleEmitEnabled = moduleKind === ModuleKind.AMD || moduleKind === ModuleKind.System;
const sourceFiles = host.getSourceFiles();
const sourceFiles = getAllEmittableSourceFiles();
// Can emit only sources that are not declaration file and are either non module code or module with --module or --target es6 specified
return filter(sourceFiles, moduleEmitEnabled ? isNonDeclarationFile : isBundleEmitNonExternalModule);
}
else {
const sourceFiles = targetSourceFile === undefined ? host.getSourceFiles() : [targetSourceFile];
const sourceFiles = targetSourceFile === undefined ? getAllEmittableSourceFiles() : [targetSourceFile];
return filter(sourceFiles, isNonDeclarationFile);
}

function getAllEmittableSourceFiles() {
return options.noEmitForJsFiles ? filter(host.getSourceFiles(), sourceFile => !isSourceFileJavaScript(sourceFile)) : host.getSourceFiles();
}
}

function isNonDeclarationFile(sourceFile: SourceFile) {
Expand Down Expand Up @@ -2589,7 +2593,7 @@ namespace ts {
}
else {
for (const sourceFile of sourceFiles) {
// Don't emit if source file is a declaration file, or was located under node_modules
// Don't emit if source file is a declaration file, or was located under node_modules
if (!isDeclarationFile(sourceFile) && !host.isSourceFileFromExternalLibrary(sourceFile)) {
onSingleFileEmit(host, sourceFile);
}
Expand Down Expand Up @@ -2651,7 +2655,7 @@ namespace ts {
onBundledEmit(host);
}
else {
const sourceFiles = targetSourceFile === undefined ? host.getSourceFiles() : [targetSourceFile];
const sourceFiles = targetSourceFile === undefined ? getSourceFilesToEmit(host) : [targetSourceFile];
for (const sourceFile of sourceFiles) {
// Don't emit if source file is a declaration file, or was located under node_modules
if (!isDeclarationFile(sourceFile) && !host.isSourceFileFromExternalLibrary(sourceFile)) {
Expand Down Expand Up @@ -2689,11 +2693,11 @@ namespace ts {
function onBundledEmit(host: EmitHost) {
// Can emit only sources that are not declaration file and are either non module code or module with
// --module or --target es6 specified. Files included by searching under node_modules are also not emitted.
const bundledSources = filter(host.getSourceFiles(),
const bundledSources = filter(getSourceFilesToEmit(host),
sourceFile => !isDeclarationFile(sourceFile) &&
!host.isSourceFileFromExternalLibrary(sourceFile) &&
(!isExternalModule(sourceFile) ||
!!getEmitModuleKind(options)));
!host.isSourceFileFromExternalLibrary(sourceFile) &&
(!isExternalModule(sourceFile) ||
!!getEmitModuleKind(options)));
if (bundledSources.length) {
const jsFilePath = options.outFile || options.out;
const emitFileNames: EmitFileNames = {
Expand Down Expand Up @@ -2879,7 +2883,7 @@ namespace ts {
writeComment: (text: string, lineMap: number[], writer: EmitTextWriter, commentPos: number, commentEnd: number, newLine: string) => void,
node: TextRange, newLine: string, removeComments: boolean) {
let leadingComments: CommentRange[];
let currentDetachedCommentInfo: {nodePos: number, detachedCommentEndPos: number};
let currentDetachedCommentInfo: { nodePos: number, detachedCommentEndPos: number };
if (removeComments) {
// removeComments is true, only reserve pinned comment at the top of file
// For example:
Expand Down Expand Up @@ -3226,10 +3230,10 @@ namespace ts {

function stringifyValue(value: any): string {
return typeof value === "string" ? `"${escapeString(value)}"`
: typeof value === "number" ? isFinite(value) ? String(value) : "null"
: typeof value === "boolean" ? value ? "true" : "false"
: typeof value === "object" && value ? isArray(value) ? cycleCheck(stringifyArray, value) : cycleCheck(stringifyObject, value)
: /*fallback*/ "null";
: typeof value === "number" ? isFinite(value) ? String(value) : "null"
: typeof value === "boolean" ? value ? "true" : "false"
: typeof value === "object" && value ? isArray(value) ? cycleCheck(stringifyArray, value) : cycleCheck(stringifyObject, value)
: /*fallback*/ "null";
}

function cycleCheck(cb: (value: any) => string, value: any) {
Expand All @@ -3254,7 +3258,7 @@ namespace ts {

function stringifyProperty(memo: string, value: any, key: string) {
return value === undefined || typeof value === "function" || key === "__cycle" ? memo
: (memo ? memo + "," : memo) + `"${escapeString(key)}":${stringifyValue(value)}`;
: (memo ? memo + "," : memo) + `"${escapeString(key)}":${stringifyValue(value)}`;
}

const base64Digits = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/=";
Expand Down Expand Up @@ -3499,7 +3503,7 @@ namespace ts {
* @param token The token.
*/
export function createTokenRange(pos: number, token: SyntaxKind): TextRange {
return createRange(pos, pos + tokenToString(token).length);
return createRange(pos, pos + tokenToString(token).length);
}

export function rangeIsOnSingleLine(range: TextRange, sourceFile: SourceFile) {
Expand Down
40 changes: 40 additions & 0 deletions src/harness/unittests/compileOnSave.ts
Original file line number Diff line number Diff line change
Expand Up @@ -492,5 +492,45 @@ namespace ts.projectSystem {
assert.isTrue(host.fileExists(expectedEmittedFileName));
assert.equal(host.readFile(expectedEmittedFileName), `"use strict";\r\nfunction Foo() { return 10; }\r\nexports.Foo = Foo;\r\n`);
});

it("shoud not emit js files in external projects", () => {
const file1 = {
path: "/a/b/file1.ts",
content: "consonle.log('file1');"
};
// file2 has errors. The emitting should not be blocked.
const file2 = {
path: "/a/b/file2.js",
content: "console.log'file2');"
};
const file3 = {
path: "/a/b/file3.js",
content: "console.log('file3');"
};
const externalProjectName = "externalproject";
const host = createServerHost([file1, file2, file3, libFile]);
const session = createSession(host);
const projectService = session.getProjectService();

projectService.openExternalProject({
rootFiles: toExternalFiles([file1.path, file2.path]),
options: {
allowJs: true,
outFile: "dist.js",
compileOnSave: true
},
projectFileName: externalProjectName
});

const emitRequest = makeSessionRequest<server.protocol.CompileOnSaveEmitFileRequestArgs>(CommandNames.CompileOnSaveEmitFile, { file: file1.path });
session.executeCommand(emitRequest);

const expectedOutFileName = "/a/b/dist.js";
assert.isTrue(host.fileExists(expectedOutFileName));
const outFileContent = host.readFile(expectedOutFileName);
assert.isTrue(outFileContent.indexOf(file1.content) !== -1);
assert.isTrue(outFileContent.indexOf(file2.content) === -1);
assert.isTrue(outFileContent.indexOf(file3.content) === -1);
});
});
}
4 changes: 2 additions & 2 deletions src/server/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,8 @@ namespace ts.server {
this.compilerOptions.allowNonTsExtensions = true;
}

if (this.projectKind === ProjectKind.Inferred) {
this.compilerOptions.noEmitOverwritenFiles = true;
if (this.projectKind === ProjectKind.Inferred || this.projectKind === ProjectKind.External) {
this.compilerOptions.noEmitForJsFiles = true;
}

if (languageServiceEnabled) {
Expand Down