From 5dfa833276ba4e30a4781894fdacf69aa7562712 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Sat, 27 Jun 2020 07:56:10 +0200 Subject: [PATCH] fix(cdk/schematics): errors when attempting to read some files In some cases the schematics fail to read a file which results in an error, because we weren't guarding against it properly. We had the correct types in place, but the tsconfig didn't have `strictNullChecks` enabled which prevented the compiler from reporting the errors. Fixes #19779. --- .../component-resource-collector.ts | 36 +++++++++++-------- src/cdk/schematics/update-tool/index.ts | 20 ++++++----- .../schematics/update-tool/target-version.ts | 5 +-- src/cdk/schematics/update-tool/tsconfig.json | 1 + .../schematics/update-tool/version-changes.ts | 10 ++---- 5 files changed, 39 insertions(+), 33 deletions(-) diff --git a/src/cdk/schematics/update-tool/component-resource-collector.ts b/src/cdk/schematics/update-tool/component-resource-collector.ts index 08ff7000cde2..8509edfc0808 100644 --- a/src/cdk/schematics/update-tool/component-resource-collector.ts +++ b/src/cdk/schematics/update-tool/component-resource-collector.ts @@ -133,13 +133,11 @@ export class ComponentResourceCollector { property.initializer.elements.forEach(el => { if (ts.isStringLiteralLike(el)) { const stylesheetPath = this._fileSystem.resolve(sourceFileDirPath, el.text); + const stylesheet = this.resolveExternalStylesheet(stylesheetPath, node); - // In case the stylesheet does not exist in the file system, skip it gracefully. - if (!this._fileSystem.exists(stylesheetPath)) { - return; + if (stylesheet) { + this.resolvedStylesheets.push(stylesheet); } - - this.resolvedStylesheets.push(this.resolveExternalStylesheet(stylesheetPath, node)); } }); } @@ -155,24 +153,32 @@ export class ComponentResourceCollector { } const fileContent = this._fileSystem.read(templatePath); - const lineStartsMap = computeLineStartsMap(fileContent); - this.resolvedTemplates.push({ - filePath: templatePath, - container: node, - content: fileContent, - inline: false, - start: 0, - getCharacterAndLineOfPosition: pos => getLineAndCharacterFromPosition(lineStartsMap, pos), - }); + if (fileContent) { + const lineStartsMap = computeLineStartsMap(fileContent); + + this.resolvedTemplates.push({ + filePath: templatePath, + container: node, + content: fileContent, + inline: false, + start: 0, + getCharacterAndLineOfPosition: p => getLineAndCharacterFromPosition(lineStartsMap, p), + }); + } } }); } /** Resolves an external stylesheet by reading its content and computing line mappings. */ resolveExternalStylesheet(filePath: WorkspacePath, container: ts.ClassDeclaration|null): - ResolvedResource { + ResolvedResource|null { const fileContent = this._fileSystem.read(filePath); + + if (!fileContent) { + return null; + } + const lineStartsMap = computeLineStartsMap(fileContent); return { diff --git a/src/cdk/schematics/update-tool/index.ts b/src/cdk/schematics/update-tool/index.ts index 99b3efeb7b98..4b98555abb77 100644 --- a/src/cdk/schematics/update-tool/index.ts +++ b/src/cdk/schematics/update-tool/index.ts @@ -102,15 +102,17 @@ export class UpdateProject { // specified in any Angular component. Therefore we allow for additional stylesheets // being specified. We visit them in each migration unless they have been already // discovered before as actual component resource. - additionalStylesheetPaths.forEach(filePath => { - const resolvedPath = this._fileSystem.resolve(filePath); - const stylesheet = resourceCollector.resolveExternalStylesheet(resolvedPath, null); - // Do not visit stylesheets which have been referenced from a component. - if (!this._analyzedFiles.has(resolvedPath)) { - migrations.forEach(r => r.visitStylesheet(stylesheet)); - this._analyzedFiles.add(resolvedPath); - } - }); + if (additionalStylesheetPaths) { + additionalStylesheetPaths.forEach(filePath => { + const resolvedPath = this._fileSystem.resolve(filePath); + const stylesheet = resourceCollector.resolveExternalStylesheet(resolvedPath, null); + // Do not visit stylesheets which have been referenced from a component. + if (!this._analyzedFiles.has(resolvedPath) && stylesheet) { + migrations.forEach(r => r.visitStylesheet(stylesheet)); + this._analyzedFiles.add(resolvedPath); + } + }); + } // Call the "postAnalysis" method for each migration. migrations.forEach(r => r.postAnalysis()); diff --git a/src/cdk/schematics/update-tool/target-version.ts b/src/cdk/schematics/update-tool/target-version.ts index 83af430e1b3a..26c26b718449 100644 --- a/src/cdk/schematics/update-tool/target-version.ts +++ b/src/cdk/schematics/update-tool/target-version.ts @@ -22,6 +22,7 @@ export enum TargetVersion { * based on the "TargetVersion" enum. */ export function getAllVersionNames(): string[] { - return Object.keys(TargetVersion) - .filter(enumValue => typeof TargetVersion[enumValue] === 'string'); + return Object.keys(TargetVersion).filter(enumValue => { + return typeof (TargetVersion as Record)[enumValue] === 'string'; + }); } diff --git a/src/cdk/schematics/update-tool/tsconfig.json b/src/cdk/schematics/update-tool/tsconfig.json index e6d7a1a309da..c553c223f3dd 100644 --- a/src/cdk/schematics/update-tool/tsconfig.json +++ b/src/cdk/schematics/update-tool/tsconfig.json @@ -1,5 +1,6 @@ { "compilerOptions": { + "strict": true, "lib": ["es2015"], "types": ["node", "glob"] } diff --git a/src/cdk/schematics/update-tool/version-changes.ts b/src/cdk/schematics/update-tool/version-changes.ts index c5d1b8c16bcb..294da5825613 100644 --- a/src/cdk/schematics/update-tool/version-changes.ts +++ b/src/cdk/schematics/update-tool/version-changes.ts @@ -29,15 +29,11 @@ export type ValueOfChanges = T extends VersionChanges? X : null; */ export function getChangesForTarget(target: TargetVersion, data: VersionChanges): T[] { if (!data) { - throw new Error( - `No data could be found for target version: ${TargetVersion[target]}`); + const version = (TargetVersion as Record)[target]; + throw new Error(`No data could be found for target version: ${version}`); } - if (!data[target]) { - return []; - } - - return data[target]!.reduce((result, prData) => result.concat(prData.changes), [] as T[]); + return (data[target] || []).reduce((result, prData) => result.concat(prData.changes), [] as T[]); } /**