Skip to content

Commit

Permalink
fix: dispose the previous TransformationResult after inlining (#533)
Browse files Browse the repository at this point in the history
Beside, re-write the `transformComponent` and `ComponenTransformer` APIs. They might become public.
  • Loading branch information
dherges authored Jan 24, 2018
1 parent 33b8e80 commit b4c7e89
Show file tree
Hide file tree
Showing 6 changed files with 127 additions and 88 deletions.
6 changes: 0 additions & 6 deletions src/lib/ng-package-format/artefacts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 => <ts.SourceFile>this.extras(key));
}

public template(file: string): string;
public template(file: string, content: string);
public template(file: string, content?: string): string | undefined {
Expand Down
4 changes: 2 additions & 2 deletions src/lib/steps/entry-point-transforms.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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');
Expand Down
27 changes: 11 additions & 16 deletions src/lib/steps/ngc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
});
Expand All @@ -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();
};

/**
Expand All @@ -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
Expand Down
132 changes: 85 additions & 47 deletions src/lib/ts/ng-component-transformer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<ts.SourceFile>;

export const componentTransformer: ComponentTransformer = ({
templateProcessor,
stylesheetProcessor,
sourceFileWriter
}) =>
}: {
template: TemplateTransformer;
stylesheet: StylesheetTransformer;
}
): ts.TransformerFactory<ts.SourceFile>;
}

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)
Expand All @@ -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;
}
});
16 changes: 8 additions & 8 deletions src/lib/ts/synthesized-compiler-host.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
30 changes: 21 additions & 9 deletions src/lib/ts/transform-component.ts
Original file line number Diff line number Diff line change
@@ -1,24 +1,33 @@
import * as ts from 'typescript';
import { isComponentDecorator, isTemplateUrl, isStyleUrls } from './ng-type-guards';

export type ComponentVisitors = {
templateVisitor: ts.Transformer<ts.PropertyAssignment>;
stylesheetVisitor: ts.Transformer<ts.PropertyAssignment>;
/**
* 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<ts.PropertyAssignment>;

/** TypeScript transformer to update the property assignment for `styleUrls: []`. */
stylesheetUrl: ts.Transformer<ts.PropertyAssignment>;

/** TypeScript transformer to update the source file. */
file?: ts.Transformer<ts.SourceFile>;
};

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;
}

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);
Expand All @@ -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));
};

0 comments on commit b4c7e89

Please sign in to comment.