Skip to content

Commit

Permalink
fix(cdk/schematics): errors when attempting to read some files
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
crisbeto committed Jun 27, 2020
1 parent 2a97418 commit 8b98f30
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 36 deletions.
36 changes: 21 additions & 15 deletions src/cdk/schematics/update-tool/component-resource-collector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
});
}
Expand All @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions src/cdk/schematics/update-tool/file-system.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,11 @@ export abstract class FileSystem<T = WorkspacePath> {
/** Applies all changes which have been recorded in update recorders. */
abstract commitEdits(): void;
/** Creates a new file with the given content. */
abstract create(filePath: T, content: string);
abstract create(filePath: T, content: string): void;
/** Overwrites an existing file with the given content. */
abstract overwrite(filePath: T, content: string);
abstract overwrite(filePath: T, content: string): void;
/** Deletes the given file. */
abstract delete(filePath: T);
abstract delete(filePath: T): void;
/**
* Resolves given paths to a resolved path in the file system. For example, the devkit
* tree considers the actual workspace directory as file system root.
Expand Down
20 changes: 11 additions & 9 deletions src/cdk/schematics/update-tool/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,15 +102,17 @@ export class UpdateProject<Context> {
// 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());
Expand Down
5 changes: 3 additions & 2 deletions src/cdk/schematics/update-tool/target-version.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string|undefined>)[enumValue] === 'string';
});
}
1 change: 1 addition & 0 deletions src/cdk/schematics/update-tool/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{
"compilerOptions": {
"strict": true,
"lib": ["es2015"],
"types": ["node", "glob"]
}
Expand Down
10 changes: 3 additions & 7 deletions src/cdk/schematics/update-tool/version-changes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,11 @@ export type ValueOfChanges<T> = T extends VersionChanges<infer X>? X : null;
*/
export function getChangesForTarget<T>(target: TargetVersion, data: VersionChanges<T>): T[] {
if (!data) {
throw new Error(
`No data could be found for target version: ${TargetVersion[target]}`);
const version = (TargetVersion as Record<string, string>)[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[]);
}

/**
Expand Down

0 comments on commit 8b98f30

Please sign in to comment.