From b4c7e8965ff74549f307397ba76f404ecc55e2ba Mon Sep 17 00:00:00 2001 From: David Herges Date: Wed, 24 Jan 2018 22:23:22 +0100 Subject: [PATCH] fix: dispose the previous TransformationResult after inlining (#533) Beside, re-write the `transformComponent` and `ComponenTransformer` APIs. They might become public. --- src/lib/ng-package-format/artefacts.ts | 6 -- src/lib/steps/entry-point-transforms.ts | 4 +- src/lib/steps/ngc.ts | 27 ++--- src/lib/ts/ng-component-transformer.ts | 132 +++++++++++++++--------- src/lib/ts/synthesized-compiler-host.ts | 16 +-- src/lib/ts/transform-component.ts | 30 ++++-- 6 files changed, 127 insertions(+), 88 deletions(-) diff --git a/src/lib/ng-package-format/artefacts.ts b/src/lib/ng-package-format/artefacts.ts index 95b27730d..4c72fcb33 100644 --- a/src/lib/ng-package-format/artefacts.ts +++ b/src/lib/ng-package-format/artefacts.ts @@ -52,12 +52,6 @@ export class NgArtefacts { this.extras('tsSources', value); } - public tsSyntheticSourcFiles(): ts.SourceFile[] { - return Array.from(this._extras.keys()) - .filter(key => key.startsWith('ts:')) - .map(key => this.extras(key)); - } - public template(file: string): string; public template(file: string, content: string); public template(file: string, content?: string): string | undefined { diff --git a/src/lib/steps/entry-point-transforms.ts b/src/lib/steps/entry-point-transforms.ts index e9a2a4ad2..fa432bc54 100644 --- a/src/lib/steps/entry-point-transforms.ts +++ b/src/lib/steps/entry-point-transforms.ts @@ -6,7 +6,7 @@ import { NgPackage } from '../ng-package-format/package'; import { BuildStep } from '../deprecations'; import { writePackage } from '../steps/package'; import { processAssets } from '../steps/assets'; -import { ngc, collectTemplateAndStylesheetFiles, inlineTemplatesAndStyles } from '../steps/ngc'; +import { ngc, extractTemplateAndStylesheetFiles, inlineTemplatesAndStyles } from '../steps/ngc'; import { FlattenOpts, writeFlatBundleFiles } from '../flatten/flatten'; import { relocateSourceMaps } from '../sourcemaps/relocate'; import { copySourceFilesToDestination } from '../steps/transfer'; @@ -35,7 +35,7 @@ export function transformSourcesFactory(prepareTsConfig: BuildStep) { // First pass: collect templateUrl and styleUrls referencing source files. log.info('Extracting templateUrl and styleUrls'); - collectTemplateAndStylesheetFiles(args); + extractTemplateAndStylesheetFiles(args); // Then, process assets keeping transformed contents in memory. log.info('Processing assets'); diff --git a/src/lib/steps/ngc.ts b/src/lib/steps/ngc.ts index 1d5b5cf23..80d0bcf89 100644 --- a/src/lib/steps/ngc.ts +++ b/src/lib/steps/ngc.ts @@ -41,14 +41,14 @@ const transformSources = ( }; /** Extracts templateUrl and styleUrls from `@Component({..})` decorators. */ -export const collectTemplateAndStylesheetFiles: BuildStep = ({ artefacts, entryPoint, pkg }) => { +export const extractTemplateAndStylesheetFiles: BuildStep = ({ artefacts, entryPoint, pkg }) => { const tsConfig = artefacts.tsConfig; const collector = componentTransformer({ - templateProcessor: (a, b, templateFilePath) => { + template: ({ templateFilePath }) => { artefacts.template(templateFilePath, null); }, - stylesheetProcessor: (a, b, styleFilePath) => { + stylesheet: ({ styleFilePath }) => { artefacts.stylesheet(styleFilePath, null); } }); @@ -60,17 +60,15 @@ export const collectTemplateAndStylesheetFiles: BuildStep = ({ artefacts, entryP export const inlineTemplatesAndStyles: BuildStep = ({ artefacts, entryPoint, pkg }) => { // inline contents from artefacts set (generated in a previous step) const transformer = componentTransformer({ - templateProcessor: (a, b, templateFilePath) => artefacts.template(templateFilePath) || '', - stylesheetProcessor: (a, b, styleFilePath) => artefacts.stylesheet(styleFilePath) || '', - sourceFileWriter: (sourceFile, node, synthesizedSourceText) => { - const key = `ts:${sourceFile.fileName}`; - const synthesizedSourceFile = replaceWithSynthesizedSourceText(node, synthesizedSourceText); - - artefacts.extras(key, synthesizedSourceFile); - } + template: ({ templateFilePath }) => artefacts.template(templateFilePath) || '', + stylesheet: ({ styleFilePath }) => artefacts.stylesheet(styleFilePath) || '' }); - artefacts.tsSources = ts.transform(artefacts.tsSources.transformed, [transformer]); + const preTransform = artefacts.tsSources; + + artefacts.tsSources = ts.transform([...artefacts.tsSources.transformed], [transformer]); + + preTransform.dispose(); }; /** @@ -82,14 +80,11 @@ export const inlineTemplatesAndStyles: BuildStep = ({ artefacts, entryPoint, pkg export async function ngc(entryPoint: NgEntryPoint, artefacts: NgArtefacts) { log.debug(`ngc (v${ng.VERSION.full}): ${entryPoint.entryFile}`); - // XX ... hacky - const mixedSourceFiles = [...artefacts.tsSyntheticSourcFiles(), ...artefacts.tsSources.transformed]; - // ng.CompilerHost const tsConfig = artefacts.tsConfig; const ngCompilerHost = ng.createCompilerHost({ options: tsConfig.options, - tsHost: createCompilerHostForSynthesizedSourceFiles(mixedSourceFiles, artefacts.tsConfig.options) + tsHost: createCompilerHostForSynthesizedSourceFiles(artefacts.tsSources.transformed, artefacts.tsConfig.options) }); // ng.Program diff --git a/src/lib/ts/ng-component-transformer.ts b/src/lib/ts/ng-component-transformer.ts index c08a52339..d5af7d7c6 100644 --- a/src/lib/ts/ng-component-transformer.ts +++ b/src/lib/ts/ng-component-transformer.ts @@ -2,68 +2,92 @@ import * as ts from 'typescript'; import * as path from 'path'; import { isComponentDecorator, isTemplateUrl, isStyleUrls } from './ng-type-guards'; import { transformComponent } from './transform-component'; +import { isSynthesizedSourceFile, replaceWithSynthesizedSourceText, writeSourceFile } from './synthesized-source-file'; + +/** + * Call signature for a transformer applied to `@Component({ templateUrl: '...' })`. + * + * A `TemplateTransformer` will update the property assignment for `templateUrl` in the decorator. + */ +export interface TemplateTransformer { + ( + { + + }: { + node: ts.Node; + sourceFile: ts.SourceFile; + sourceFilePath: string; + templatePath: string; + templateFilePath: string; + } + ): string | undefined | void; +} + +/** + * Call signature for a transformer applied to `@Component({ styleUrls: ['...'] })`. + * + * A `StylesheetTransformer` will update the property assignment for `stylesUrl` in the decorator. + * + * WATCH OUT! A stylesheet transformer is called for every url in the `stylesUrl` array! + */ +export interface StylesheetTransformer { + ( + { + + }: { + node: ts.Node; + sourceFile: ts.SourceFile; + sourceFilePath: string; + stylePath: string; + styleFilePath: string; + } + ): string | undefined | void; +} + +export interface ComponentTransformer { + ( + { -export type StylesheetProcessor = ( - sourceFile: string, - styleUrl: string, - styleFilePath: string -) => string | undefined | void; - -export type TemplateProcessor = ( - sourceFile: string, - templateUrl: string, - templateFilePath: string -) => string | undefined | void; - -export type SourceFileWriter = (sourceFile: ts.SourceFile, node: ts.Node, sourceText: string) => void; - -export type ComponentTransformer = ( - { - - }: { - templateProcessor: TemplateProcessor; - stylesheetProcessor: StylesheetProcessor; - sourceFileWriter?: any; - } -) => ts.TransformerFactory; - -export const componentTransformer: ComponentTransformer = ({ - templateProcessor, - stylesheetProcessor, - sourceFileWriter -}) => + }: { + template: TemplateTransformer; + stylesheet: StylesheetTransformer; + } + ): ts.TransformerFactory; +} + +export const componentTransformer: ComponentTransformer = ({ template, stylesheet }) => transformComponent({ - templateVisitor: node => { + templateUrl: node => { const sourceFile = node.getSourceFile(); const sourceFilePath = node.getSourceFile().fileName; - // XX: strip quotes (' or ") from path const templatePath = node.initializer.getText().substring(1, node.initializer.getText().length - 1); const templateFilePath = path.resolve(path.dirname(sourceFilePath), templatePath); - const template = templateProcessor(sourceFilePath, templatePath, templateFilePath); - if (typeof template === 'string') { + // Call the transformer + const inlinedTemplate = template({ node, sourceFile, sourceFilePath, templatePath, templateFilePath }); + + if (typeof inlinedTemplate === 'string') { + // Apply the transformer result, thus altering the source file const synthesizedNode = ts.updatePropertyAssignment( node, ts.createIdentifier('template'), - ts.createLiteral(template) + ts.createLiteral(inlinedTemplate) ); - if (sourceFileWriter) { - const synthesizedSourceText = 'template: `'.concat(template).concat('`'); - sourceFileWriter(sourceFile, node, synthesizedSourceText); - } + const synthesizedSourceText = 'template: `'.concat(inlinedTemplate).concat('`'); + replaceWithSynthesizedSourceText(node, synthesizedSourceText); return synthesizedNode; } else { return node; } }, - stylesheetVisitor: node => { + stylesheetUrl: node => { const sourceFile = node.getSourceFile(); const sourceFilePath = node.getSourceFile().fileName; - // handle array arguments for styleUrls + // Handle array arguments for styleUrls const styleUrls = node.initializer .getChildren() .filter(node => node.kind === ts.SyntaxKind.SyntaxList) @@ -72,34 +96,48 @@ export const componentTransformer: ComponentTransformer = ({ .filter(text => text !== ',') .map(url => url.substring(1, url.length - 1)); + // Call the transformation for each value found in `stylesUrls: []`. const stylesheets = styleUrls.map((url: string) => { const styleFilePath = path.resolve(path.dirname(sourceFilePath), url); - const content = stylesheetProcessor(sourceFilePath, url, styleFilePath); + + // Call the stylesheet transformer + const content = stylesheet({ node, sourceFile, sourceFilePath, stylePath: url, styleFilePath }); return typeof content === 'string' ? content : url; }); + // Check if the transformer manipulated the metadata of the `@Component({..})` decorator const hasChanged = stylesheets.every((value, index) => { return styleUrls[index] && styleUrls[index] !== value; }); if (hasChanged) { + // Apply the transformation result, thus altering the source file const synthesizedNode = ts.updatePropertyAssignment( node, ts.createIdentifier('styles'), ts.createArrayLiteral(stylesheets.map(value => ts.createLiteral(value))) ); - if (sourceFileWriter) { - const synthesizedSourceText = 'styles: [' - .concat(stylesheets.map(value => `\`${value}\``).join(', ')) - .concat(']'); - sourceFileWriter(sourceFile, node, synthesizedSourceText); - } + const synthesizedSourceText = 'styles: [' + .concat(stylesheets.map(value => `\`${value}\``).join(', ')) + .concat(']'); + replaceWithSynthesizedSourceText(node, synthesizedSourceText); return synthesizedNode; } else { return node; } + }, + file: sourceFile => { + // XX ... the string replacement is quite hacky. + // Why can't we use `ts.SourceFile#update()`? + // It produces a `FalseExpression` error, somehow. + if (isSynthesizedSourceFile(sourceFile['original'])) { + sourceFile['__replacements'] = sourceFile['original'].__replacements; + return writeSourceFile(sourceFile); + } + + return sourceFile; } }); diff --git a/src/lib/ts/synthesized-compiler-host.ts b/src/lib/ts/synthesized-compiler-host.ts index 6ae6785a9..066d2f56e 100644 --- a/src/lib/ts/synthesized-compiler-host.ts +++ b/src/lib/ts/synthesized-compiler-host.ts @@ -18,21 +18,21 @@ export function createCompilerHostForSynthesizedSourceFiles( return { ...wrapped, getSourceFile: (fileName, version) => { - const fromCollection = sourceFiles.find(file => file.fileName === fileName); + const sourceFile = sourceFiles.find(file => file.fileName === fileName); - if (fromCollection) { + if (sourceFile) { // FIX @link https://github.com/Microsoft/TypeScript/issues/19950 - if (!fromCollection['ambientModuleNames']) { - fromCollection['ambientModuleNames'] = fromCollection['original']['ambientModuleNames']; + if (!sourceFile['ambientModuleNames'] && sourceFile['original']) { + sourceFile['ambientModuleNames'] = sourceFile['original']['ambientModuleNames']; } // FIX synthesized source files cause ngc/tsc/tsickle to chock - const hasSyntheticFlag = (fromCollection.flags & 8) !== 0; - if (hasSyntheticFlag || isSynthesizedSourceFile(fromCollection)) { - return writeSourceFile(fromCollection); + const hasSyntheticFlag = (sourceFile.flags & 8) !== 0; + if (hasSyntheticFlag || isSynthesizedSourceFile(sourceFile)) { + return writeSourceFile(sourceFile); } - return fromCollection; + return sourceFile; } else { return wrapped.getSourceFile(fileName, version); } diff --git a/src/lib/ts/transform-component.ts b/src/lib/ts/transform-component.ts index 4ee212f1c..49d5a10af 100644 --- a/src/lib/ts/transform-component.ts +++ b/src/lib/ts/transform-component.ts @@ -1,14 +1,23 @@ import * as ts from 'typescript'; import { isComponentDecorator, isTemplateUrl, isStyleUrls } from './ng-type-guards'; -export type ComponentVisitors = { - templateVisitor: ts.Transformer; - stylesheetVisitor: ts.Transformer; +/** + * A transformer that updates the metadata for Angular `@Component({})` decorators. + */ +export type ComponentTransformer = { + /** TypeScript transformer to update the property assignment for `templateUrl: '..'`. */ + templateUrl: ts.Transformer; + + /** TypeScript transformer to update the property assignment for `styleUrls: []`. */ + stylesheetUrl: ts.Transformer; + + /** TypeScript transformer to update the source file. */ + file?: ts.Transformer; }; -export const transformComponent = ({ templateVisitor, stylesheetVisitor }: ComponentVisitors) => ( - context: ts.TransformationContext -) => (sourceFile: ts.SourceFile): ts.SourceFile => { +export const transformComponent = (transformer: ComponentTransformer) => (context: ts.TransformationContext) => ( + sourceFile: ts.SourceFile +): ts.SourceFile => { // skip source files from 'node_modules' directory (third-party source) if (sourceFile.fileName.includes('node_modules')) { return sourceFile; @@ -16,9 +25,9 @@ export const transformComponent = ({ templateVisitor, stylesheetVisitor }: Compo const visitComponentMetadata: ts.Visitor = (node: ts.Decorator) => { if (isTemplateUrl(node)) { - return templateVisitor(node); + return transformer.templateUrl(node); } else if (isStyleUrls(node)) { - return stylesheetVisitor(node); + return transformer.stylesheetUrl(node); } return ts.visitEachChild(node, visitComponentMetadata, context); @@ -29,5 +38,8 @@ export const transformComponent = ({ templateVisitor, stylesheetVisitor }: Compo ? ts.visitEachChild(node, visitComponentMetadata, context) : ts.visitEachChild(node, visitDecorators, context); - return ts.visitNode(sourceFile, visitDecorators); + // Either custom file transformer or identity transform + const transformFile = transformer.file ? transformer.file : file => file; + + return transformFile(ts.visitNode(sourceFile, visitDecorators)); };