From f31f7f99d3d7bf43d7226c21a9e69cd56f010017 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Sun, 23 Dec 2018 17:57:12 +0100 Subject: [PATCH 1/2] build: ensure that there are no invalid dynamic imports References https://github.com/angular/material2/issues/12877 --- .../release/release-output/check-packages.ts | 19 ++++++++- .../release-output/output-validations.ts | 42 ++++++++++++++++++- 2 files changed, 57 insertions(+), 4 deletions(-) diff --git a/tools/release/release-output/check-packages.ts b/tools/release/release-output/check-packages.ts index eed335a3e0ad..b7ade6d40d7e 100644 --- a/tools/release/release-output/check-packages.ts +++ b/tools/release/release-output/check-packages.ts @@ -2,11 +2,18 @@ import {bold, red, yellow} from 'chalk'; import {existsSync} from 'fs'; import {sync as glob} from 'glob'; import {join} from 'path'; -import {checkMaterialPackage, checkReleaseBundle} from './output-validations'; +import { + checkMaterialPackage, + checkReleaseBundle, + checkTypeDefinitionFile +} from './output-validations'; /** Glob that matches all JavaScript bundle files within a release package. */ const releaseBundlesGlob = '+(esm5|esm2015|bundles)/*.js'; +/** Glob that matches all TypeScript definition files within a release package. */ +const releaseTypeDefinitionsGlob = '**/*.d.ts'; + /** * Type that describes a map of package failures. The keys are failure messages and * their value is an array of specifically affected files. @@ -21,12 +28,14 @@ type PackageFailures = Map; */ export function checkReleasePackage(releasesPath: string, packageName: string): boolean { const packagePath = join(releasesPath, packageName); - const bundlePaths = glob(releaseBundlesGlob, {cwd: packagePath, absolute: true}); const failures = new Map() as PackageFailures; const addFailure = (message, filePath?) => { failures.set(message, (failures.get(message) || []).concat(filePath)); }; + const bundlePaths = glob(releaseBundlesGlob, {cwd: packagePath, absolute: true}); + const typeDefinitions = glob(releaseTypeDefinitionsGlob, {cwd: packagePath, absolute: true}); + // We want to walk through each bundle within the current package and run // release validations that ensure that the bundles are not invalid. bundlePaths.forEach(bundlePath => { @@ -34,6 +43,12 @@ export function checkReleasePackage(releasesPath: string, packageName: string): .forEach(message => addFailure(message, bundlePath)); }); + // Run output validations for all TypeScript definition files within the release output. + typeDefinitions.forEach(filePath => { + checkTypeDefinitionFile(filePath) + .forEach(message => addFailure(message, filePath)); + }); + // Special release validation checks for the "material" release package. if (packageName === 'material') { checkMaterialPackage(join(releasesPath, packageName)) diff --git a/tools/release/release-output/output-validations.ts b/tools/release/release-output/output-validations.ts index e8bd7ebdd8d9..6423a4f2789a 100644 --- a/tools/release/release-output/output-validations.ts +++ b/tools/release/release-output/output-validations.ts @@ -1,6 +1,6 @@ import {existsSync, readFileSync} from 'fs'; import {sync as glob} from 'glob'; -import {join} from 'path'; +import {dirname, isAbsolute, join} from 'path'; /** RegExp that matches Angular component inline styles that contain a sourcemap reference. */ const inlineStylesSourcemapRegex = /styles: ?\[["'].*sourceMappingURL=.*["']/; @@ -8,13 +8,16 @@ const inlineStylesSourcemapRegex = /styles: ?\[["'].*sourceMappingURL=.*["']/; /** RegExp that matches Angular component metadata properties that refer to external resources. */ const externalReferencesRegex = /(templateUrl|styleUrls): *["'[]/; +/** RegExp that matches TypeScript dynamic import statements. */ +const dynamicImportStatement = /import\(["'](.*)["']\)/g; + /** * Checks the specified release bundle and ensures that it does not contain * any external resource URLs. */ export function checkReleaseBundle(bundlePath: string): string[] { const bundleContent = readFileSync(bundlePath, 'utf8'); - let failures: string[] = []; + const failures: string[] = []; if (inlineStylesSourcemapRegex.exec(bundleContent) !== null) { failures.push('Found sourcemap references in component styles.'); @@ -27,6 +30,41 @@ export function checkReleaseBundle(bundlePath: string): string[] { return failures; } +/** + * Checks the specified TypeScript definition file by ensuring it does not contain invalid + * dynamic import statements. There can be invalid dynamic imports paths because we compose the + * release package by moving things in a desired output structure. See Angular package format + * specification and https://github.com/angular/material2/pull/12876 + */ +export function checkTypeDefinitionFile(filePath: string): string[] { + const baseDir = dirname(filePath); + const fileContent = readFileSync(filePath, 'utf8'); + const failures = []; + + let match: RegExpMatchArray; + + // Walk through each dynamic import and ensure that the import path is valid within the release + // output. Note that we don't want to enforce that there are no dynamic imports because type + // inference is heavily used within the schematics and is useful in some situations. + while ((match = dynamicImportStatement.exec(fileContent)) !== null) { + const importPath = match[1]; + + if (isAbsolute(importPath)) { + failures.push('Found absolute dynamic imports in definition file.'); + continue; + } + + // In case the dynamic import path starts with a dot, we know that this is a relative path + // and can ensure that the target path exists. Note that we cannot completely rely on + // "isAbsolute" because dynamic imports can also import from modules (e.g. "my-npm-module") + if (importPath.startsWith('.') && !existsSync(join(baseDir, `${importPath}.d.ts`))) { + failures.push('Found relative dynamic imports which do not exist.'); + } + } + + return failures; +} + /** * Checks the Angular Material release package and ensures that prebuilt themes * and the theming bundle are built properly. From 23a6d5af332cd2fa63a588fef53bc498a409b60c Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Mon, 24 Dec 2018 11:07:03 +0100 Subject: [PATCH 2/2] Use TypeScript AST for parsing --- .../release-output/output-validations.ts | 45 ++++++++++--------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/tools/release/release-output/output-validations.ts b/tools/release/release-output/output-validations.ts index 6423a4f2789a..2e76e956ad55 100644 --- a/tools/release/release-output/output-validations.ts +++ b/tools/release/release-output/output-validations.ts @@ -1,6 +1,7 @@ import {existsSync, readFileSync} from 'fs'; import {sync as glob} from 'glob'; import {dirname, isAbsolute, join} from 'path'; +import * as ts from 'typescript'; /** RegExp that matches Angular component inline styles that contain a sourcemap reference. */ const inlineStylesSourcemapRegex = /styles: ?\[["'].*sourceMappingURL=.*["']/; @@ -8,9 +9,6 @@ const inlineStylesSourcemapRegex = /styles: ?\[["'].*sourceMappingURL=.*["']/; /** RegExp that matches Angular component metadata properties that refer to external resources. */ const externalReferencesRegex = /(templateUrl|styleUrls): *["'[]/; -/** RegExp that matches TypeScript dynamic import statements. */ -const dynamicImportStatement = /import\(["'](.*)["']\)/g; - /** * Checks the specified release bundle and ensures that it does not contain * any external resource URLs. @@ -32,7 +30,7 @@ export function checkReleaseBundle(bundlePath: string): string[] { /** * Checks the specified TypeScript definition file by ensuring it does not contain invalid - * dynamic import statements. There can be invalid dynamic imports paths because we compose the + * dynamic import statements. There can be invalid type imports paths because we compose the * release package by moving things in a desired output structure. See Angular package format * specification and https://github.com/angular/material2/pull/12876 */ @@ -41,25 +39,30 @@ export function checkTypeDefinitionFile(filePath: string): string[] { const fileContent = readFileSync(filePath, 'utf8'); const failures = []; - let match: RegExpMatchArray; - - // Walk through each dynamic import and ensure that the import path is valid within the release - // output. Note that we don't want to enforce that there are no dynamic imports because type - // inference is heavily used within the schematics and is useful in some situations. - while ((match = dynamicImportStatement.exec(fileContent)) !== null) { - const importPath = match[1]; - - if (isAbsolute(importPath)) { - failures.push('Found absolute dynamic imports in definition file.'); - continue; + const sourceFile = ts.createSourceFile(filePath, fileContent, ts.ScriptTarget.Latest, true); + const nodeQueue = [...sourceFile.getChildren()]; + + while (nodeQueue.length) { + const node = nodeQueue.shift()!; + + // Check all dynamic type imports and ensure that the import path is valid within the release + // output. Note that we don't want to enforce that there are no dynamic type imports because + // type inference is heavily used within the schematics and is useful in some situations. + if (ts.isImportTypeNode(node) && ts.isLiteralTypeNode(node.argument) && + ts.isStringLiteral(node.argument.literal)) { + const importPath = node.argument.literal.text; + + // In case the type import path starts with a dot, we know that this is a relative path + // and can ensure that the target path exists. Note that we cannot completely rely on + // "isAbsolute" because dynamic imports can also import from modules (e.g. "my-npm-module") + if (importPath.startsWith('.') && !existsSync(join(baseDir, `${importPath}.d.ts`))) { + failures.push('Found relative type imports which do not exist.'); + } else if (isAbsolute(importPath)) { + failures.push('Found absolute type imports in definition file.'); + } } - // In case the dynamic import path starts with a dot, we know that this is a relative path - // and can ensure that the target path exists. Note that we cannot completely rely on - // "isAbsolute" because dynamic imports can also import from modules (e.g. "my-npm-module") - if (importPath.startsWith('.') && !existsSync(join(baseDir, `${importPath}.d.ts`))) { - failures.push('Found relative dynamic imports which do not exist.'); - } + nodeQueue.push(...node.getChildren()); } return failures;