fix(ivy): use globally unique names for i18n constants (#25689)

Closure compiler requires that the i18n message constants of the form

const MSG_XYZ = goog.getMessage('...');

have names that are unique across an entire compilation, even if the
variables themselves are local to a given module. This means that in
practice these names must be unique in a codebase.

The best way to guarantee this requirement is met is to encode the
relative file name of the file into which the constant is being written
into the constant name itself. This commit implements that solution.

PR Close #25689
This commit is contained in:
Alex Rickabaugh 2018-08-27 11:11:02 -07:00 committed by Misko Hevery
parent bd0eb0d1d4
commit cc29b9cf93
14 changed files with 95 additions and 28 deletions

View File

@ -49,14 +49,16 @@ export class Analyzer {
handlers: DecoratorHandler<any, any>[] = [ handlers: DecoratorHandler<any, any>[] = [
new BaseDefDecoratorHandler(this.typeChecker, this.host), new BaseDefDecoratorHandler(this.typeChecker, this.host),
new ComponentDecoratorHandler( new ComponentDecoratorHandler(
this.typeChecker, this.host, this.scopeRegistry, false, this.resourceLoader), this.typeChecker, this.host, this.scopeRegistry, false, this.resourceLoader, this.rootDirs),
new DirectiveDecoratorHandler(this.typeChecker, this.host, this.scopeRegistry, false), new DirectiveDecoratorHandler(this.typeChecker, this.host, this.scopeRegistry, false),
new InjectableDecoratorHandler(this.host, false), new InjectableDecoratorHandler(this.host, false),
new NgModuleDecoratorHandler(this.typeChecker, this.host, this.scopeRegistry, false), new NgModuleDecoratorHandler(this.typeChecker, this.host, this.scopeRegistry, false),
new PipeDecoratorHandler(this.typeChecker, this.host, this.scopeRegistry, false), new PipeDecoratorHandler(this.typeChecker, this.host, this.scopeRegistry, false),
]; ];
constructor(private typeChecker: ts.TypeChecker, private host: NgccReflectionHost) {} constructor(
private typeChecker: ts.TypeChecker, private host: NgccReflectionHost,
private rootDirs: string[]) {}
/** /**
* Analyize a parsed file to generate the information about decorated classes that * Analyize a parsed file to generate the information about decorated classes that

View File

@ -65,13 +65,21 @@ export class PackageTransformer {
// Create the TS program and necessary helpers. // Create the TS program and necessary helpers.
// TODO : create a custom compiler host that reads from .bak files if available. // TODO : create a custom compiler host that reads from .bak files if available.
const host = ts.createCompilerHost(options); const host = ts.createCompilerHost(options);
let rootDirs: string[]|undefined = undefined;
if (options.rootDirs !== undefined) {
rootDirs = options.rootDirs;
} else if (options.rootDir !== undefined) {
rootDirs = [options.rootDir];
} else {
rootDirs = [host.getCurrentDirectory()];
}
const packageProgram = ts.createProgram([entryPoint.entryFileName], options, host); const packageProgram = ts.createProgram([entryPoint.entryFileName], options, host);
const typeChecker = packageProgram.getTypeChecker(); const typeChecker = packageProgram.getTypeChecker();
const dtsMapper = new DtsMapper(entryPoint.entryRoot, entryPoint.dtsEntryRoot); const dtsMapper = new DtsMapper(entryPoint.entryRoot, entryPoint.dtsEntryRoot);
const reflectionHost = this.getHost(format, packageProgram, dtsMapper); const reflectionHost = this.getHost(format, packageProgram, dtsMapper);
const parser = this.getFileParser(format, packageProgram, reflectionHost); const parser = this.getFileParser(format, packageProgram, reflectionHost);
const analyzer = new Analyzer(typeChecker, reflectionHost); const analyzer = new Analyzer(typeChecker, reflectionHost, rootDirs);
const renderer = this.getRenderer(format, packageProgram, reflectionHost); const renderer = this.getRenderer(format, packageProgram, reflectionHost);
// Parse and analyze the files. // Parse and analyze the files.

View File

@ -80,7 +80,7 @@ describe('Analyzer', () => {
program = makeProgram(TEST_PROGRAM); program = makeProgram(TEST_PROGRAM);
const file = createParsedFile(program); const file = createParsedFile(program);
const analyzer = new Analyzer( const analyzer = new Analyzer(
program.getTypeChecker(), new Fesm2015ReflectionHost(program.getTypeChecker())); program.getTypeChecker(), new Fesm2015ReflectionHost(program.getTypeChecker()), ['']);
testHandler = createTestHandler(); testHandler = createTestHandler();
analyzer.handlers = [testHandler]; analyzer.handlers = [testHandler];
result = analyzer.analyzeFile(file); result = analyzer.analyzeFile(file);

View File

@ -17,7 +17,7 @@ function setup(file: {name: string, contents: string}) {
const program = makeProgram(file); const program = makeProgram(file);
const host = new Fesm2015ReflectionHost(program.getTypeChecker()); const host = new Fesm2015ReflectionHost(program.getTypeChecker());
const parser = new Esm2015FileParser(program, host); const parser = new Esm2015FileParser(program, host);
const analyzer = new Analyzer(program.getTypeChecker(), host); const analyzer = new Analyzer(program.getTypeChecker(), host, ['']);
const renderer = new Esm2015Renderer(host); const renderer = new Esm2015Renderer(host);
return {analyzer, host, parser, program, renderer}; return {analyzer, host, parser, program, renderer};
} }

View File

@ -17,7 +17,7 @@ function setup(file: {name: string, contents: string}) {
const program = makeProgram(file); const program = makeProgram(file);
const host = new Esm5ReflectionHost(program.getTypeChecker()); const host = new Esm5ReflectionHost(program.getTypeChecker());
const parser = new Esm5FileParser(program, host); const parser = new Esm5FileParser(program, host);
const analyzer = new Analyzer(program.getTypeChecker(), host); const analyzer = new Analyzer(program.getTypeChecker(), host, ['']);
const renderer = new Esm5Renderer(host); const renderer = new Esm5Renderer(host);
return {analyzer, host, parser, program, renderer}; return {analyzer, host, parser, program, renderer};
} }

View File

@ -46,7 +46,7 @@ function analyze(file: {name: string, contents: string}) {
const program = makeProgram(file); const program = makeProgram(file);
const host = new Fesm2015ReflectionHost(program.getTypeChecker()); const host = new Fesm2015ReflectionHost(program.getTypeChecker());
const parser = new Esm2015FileParser(program, host); const parser = new Esm2015FileParser(program, host);
const analyzer = new Analyzer(program.getTypeChecker(), host); const analyzer = new Analyzer(program.getTypeChecker(), host, ['']);
const parsedFiles = parser.parseFile(program.getSourceFile(file.name) !); const parsedFiles = parser.parseFile(program.getSourceFile(file.name) !);
return parsedFiles.map(file => analyzer.analyzeFile(file)); return parsedFiles.map(file => analyzer.analyzeFile(file));

View File

@ -29,7 +29,7 @@ export class ComponentDecoratorHandler implements DecoratorHandler<R3ComponentMe
constructor( constructor(
private checker: ts.TypeChecker, private reflector: ReflectionHost, private checker: ts.TypeChecker, private reflector: ReflectionHost,
private scopeRegistry: SelectorScopeRegistry, private isCore: boolean, private scopeRegistry: SelectorScopeRegistry, private isCore: boolean,
private resourceLoader: ResourceLoader) {} private resourceLoader: ResourceLoader, private rootDirs: string[]) {}
private literalCache = new Map<Decorator, ts.ObjectLiteralExpression>(); private literalCache = new Map<Decorator, ts.ObjectLiteralExpression>();
@ -111,9 +111,21 @@ export class ComponentDecoratorHandler implements DecoratorHandler<R3ComponentMe
preserveWhitespaces = value; preserveWhitespaces = value;
} }
// Go through the root directories for this project, and select the one with the smallest
// relative path representation.
const filePath = node.getSourceFile().fileName;
const relativeFilePath = this.rootDirs.reduce<string|undefined>((previous, rootDir) => {
const candidate = path.posix.relative(rootDir, filePath);
if (previous === undefined || candidate.length < previous.length) {
return candidate;
} else {
return previous;
}
}, undefined) !;
const template = parseTemplate( const template = parseTemplate(
templateStr, `${node.getSourceFile().fileName}#${node.name!.text}/template.html`, templateStr, `${node.getSourceFile().fileName}#${node.name!.text}/template.html`,
{preserveWhitespaces}); {preserveWhitespaces}, relativeFilePath);
if (template.errors !== undefined) { if (template.errors !== undefined) {
throw new Error( throw new Error(
`Errors parsing template: ${template.errors.map(e => e.toString()).join(', ')}`); `Errors parsing template: ${template.errors.map(e => e.toString()).join(', ')}`);

View File

@ -39,7 +39,8 @@ describe('ComponentDecoratorHandler', () => {
const checker = program.getTypeChecker(); const checker = program.getTypeChecker();
const host = new TypeScriptReflectionHost(checker); const host = new TypeScriptReflectionHost(checker);
const handler = new ComponentDecoratorHandler( const handler = new ComponentDecoratorHandler(
checker, host, new SelectorScopeRegistry(checker, host), false, new NoopResourceLoader()); checker, host, new SelectorScopeRegistry(checker, host), false, new NoopResourceLoader(),
['']);
const TestCmp = getDeclaration(program, 'entry.ts', 'TestCmp', ts.isClassDeclaration); const TestCmp = getDeclaration(program, 'entry.ts', 'TestCmp', ts.isClassDeclaration);
const detected = handler.detect(TestCmp, host.getDecoratorsOfDeclaration(TestCmp)); const detected = handler.detect(TestCmp, host.getDecoratorsOfDeclaration(TestCmp));
if (detected === undefined) { if (detected === undefined) {

View File

@ -29,11 +29,20 @@ export class NgtscProgram implements api.Program {
private _coreImportsFrom: ts.SourceFile|null|undefined = undefined; private _coreImportsFrom: ts.SourceFile|null|undefined = undefined;
private _reflector: TypeScriptReflectionHost|undefined = undefined; private _reflector: TypeScriptReflectionHost|undefined = undefined;
private _isCore: boolean|undefined = undefined; private _isCore: boolean|undefined = undefined;
private rootDirs: string[];
constructor( constructor(
rootNames: ReadonlyArray<string>, private options: api.CompilerOptions, rootNames: ReadonlyArray<string>, private options: api.CompilerOptions,
host: api.CompilerHost, oldProgram?: api.Program) { host: api.CompilerHost, oldProgram?: api.Program) {
this.rootDirs = [];
if (options.rootDirs !== undefined) {
this.rootDirs.push(...options.rootDirs);
} else if (options.rootDir !== undefined) {
this.rootDirs.push(options.rootDir);
} else {
this.rootDirs.push(host.getCurrentDirectory());
}
this.resourceLoader = host.readResource !== undefined ? this.resourceLoader = host.readResource !== undefined ?
new HostResourceLoader(host.readResource.bind(host)) : new HostResourceLoader(host.readResource.bind(host)) :
new FileResourceLoader(); new FileResourceLoader();
@ -177,7 +186,7 @@ export class NgtscProgram implements api.Program {
const handlers = [ const handlers = [
new BaseDefDecoratorHandler(checker, this.reflector), new BaseDefDecoratorHandler(checker, this.reflector),
new ComponentDecoratorHandler( new ComponentDecoratorHandler(
checker, this.reflector, scopeRegistry, this.isCore, this.resourceLoader), checker, this.reflector, scopeRegistry, this.isCore, this.resourceLoader, this.rootDirs),
new DirectiveDecoratorHandler(checker, this.reflector, scopeRegistry, this.isCore), new DirectiveDecoratorHandler(checker, this.reflector, scopeRegistry, this.isCore),
new InjectableDecoratorHandler(this.reflector, this.isCore), new InjectableDecoratorHandler(this.reflector, this.isCore),
new NgModuleDecoratorHandler(checker, this.reflector, scopeRegistry, this.isCore), new NgModuleDecoratorHandler(checker, this.reflector, scopeRegistry, this.isCore),

View File

@ -122,7 +122,8 @@ export class ConstantPool {
// */ // */
// const MSG_XYZ = goog.getMsg('message'); // const MSG_XYZ = goog.getMsg('message');
// ``` // ```
getTranslation(message: string, meta: {description?: string, meaning?: string}): o.Expression { getTranslation(message: string, meta: {description?: string, meaning?: string}, suffix: string):
o.Expression {
// The identity of an i18n message depends on the message and its meaning // The identity of an i18n message depends on the message and its meaning
const key = meta.meaning ? `${message}\u0000\u0000${meta.meaning}` : message; const key = meta.meaning ? `${message}\u0000\u0000${meta.meaning}` : message;
@ -138,7 +139,7 @@ export class ConstantPool {
} }
// Call closure to get the translation // Call closure to get the translation
const variable = o.variable(this.freshTranslationName()); const variable = o.variable(this.freshTranslationName(suffix));
const fnCall = o.variable(GOOG_GET_MSG).callFn([o.literal(message)]); const fnCall = o.variable(GOOG_GET_MSG).callFn([o.literal(message)]);
const msgStmt = variable.set(fnCall).toDeclStmt(o.INFERRED_TYPE, [o.StmtModifier.Final]); const msgStmt = variable.set(fnCall).toDeclStmt(o.INFERRED_TYPE, [o.StmtModifier.Final]);
this.statements.push(msgStmt); this.statements.push(msgStmt);
@ -257,8 +258,8 @@ export class ConstantPool {
private freshName(): string { return this.uniqueName(CONSTANT_PREFIX); } private freshName(): string { return this.uniqueName(CONSTANT_PREFIX); }
private freshTranslationName(): string { private freshTranslationName(suffix: string): string {
return this.uniqueName(TRANSLATION_PREFIX).toUpperCase(); return this.uniqueName(TRANSLATION_PREFIX + suffix).toUpperCase();
} }
private keyOf(expression: o.Expression) { private keyOf(expression: o.Expression) {

View File

@ -127,6 +127,13 @@ export interface R3ComponentMetadata extends R3DirectiveMetadata {
* Selectors found in the <ng-content> tags in the template. * Selectors found in the <ng-content> tags in the template.
*/ */
ngContentSelectors: string[]; ngContentSelectors: string[];
/**
* Path to the .ts file in which this template's generated code will be included, relative to
* the compilation root. This will be used to generate identifiers that need to be globally
* unique in certain contexts (such as g3).
*/
relativeContextFilePath: string;
}; };
/** /**

View File

@ -209,7 +209,8 @@ export function compileComponentFromMetadata(
const template = meta.template; const template = meta.template;
const templateBuilder = new TemplateDefinitionBuilder( const templateBuilder = new TemplateDefinitionBuilder(
constantPool, BindingScope.ROOT_SCOPE, 0, templateTypeName, templateName, meta.viewQueries, constantPool, BindingScope.ROOT_SCOPE, 0, templateTypeName, templateName, meta.viewQueries,
directiveMatcher, directivesUsed, meta.pipes, pipesUsed, R3.namespaceHTML); directiveMatcher, directivesUsed, meta.pipes, pipesUsed, R3.namespaceHTML,
meta.template.relativeContextFilePath);
const templateFunctionExpression = templateBuilder.buildTemplateFunction( const templateFunctionExpression = templateBuilder.buildTemplateFunction(
template.nodes, [], template.hasNgContent, template.ngContentSelectors); template.nodes, [], template.hasNgContent, template.ngContentSelectors);
@ -309,6 +310,7 @@ export function compileComponentFromRender2(
nodes: render3Ast.nodes, nodes: render3Ast.nodes,
hasNgContent: render3Ast.hasNgContent, hasNgContent: render3Ast.hasNgContent,
ngContentSelectors: render3Ast.ngContentSelectors, ngContentSelectors: render3Ast.ngContentSelectors,
relativeContextFilePath: '',
}, },
directives: typeMapToExpressionMap(directiveTypeBySel, outputCtx), directives: typeMapToExpressionMap(directiveTypeBySel, outputCtx),
pipes: typeMapToExpressionMap(pipeTypeByName, outputCtx), pipes: typeMapToExpressionMap(pipeTypeByName, outputCtx),

View File

@ -96,18 +96,25 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
// Number of binding slots // Number of binding slots
private _bindingSlots = 0; private _bindingSlots = 0;
private fileBasedI18nSuffix: string;
constructor( constructor(
private constantPool: ConstantPool, parentBindingScope: BindingScope, private level = 0, private constantPool: ConstantPool, parentBindingScope: BindingScope, private level = 0,
private contextName: string|null, private templateName: string|null, private contextName: string|null, private templateName: string|null,
private viewQueries: R3QueryMetadata[], private directiveMatcher: SelectorMatcher|null, private viewQueries: R3QueryMetadata[], private directiveMatcher: SelectorMatcher|null,
private directives: Set<o.Expression>, private pipeTypeByName: Map<string, o.Expression>, private directives: Set<o.Expression>, private pipeTypeByName: Map<string, o.Expression>,
private pipes: Set<o.Expression>, private _namespace: o.ExternalReference) { private pipes: Set<o.Expression>, private _namespace: o.ExternalReference,
private relativeContextFilePath: string) {
// view queries can take up space in data and allocation happens earlier (in the "viewQuery" // view queries can take up space in data and allocation happens earlier (in the "viewQuery"
// function) // function)
this._dataIndex = viewQueries.length; this._dataIndex = viewQueries.length;
this._bindingScope = parentBindingScope.nestedScope(level); this._bindingScope = parentBindingScope.nestedScope(level);
// Turn the relative context file path into an identifier by replacing non-alphanumeric
// characters with underscores.
this.fileBasedI18nSuffix = relativeContextFilePath.replace(/[^A-Za-z0-9]/g, '_') + '_';
this._valueConverter = new ValueConverter( this._valueConverter = new ValueConverter(
constantPool, () => this.allocateDataSlot(), constantPool, () => this.allocateDataSlot(),
(numSlots: number) => this.allocatePureFunctionSlots(numSlots), (numSlots: number) => this.allocatePureFunctionSlots(numSlots),
@ -385,7 +392,7 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
attributes.push(o.literal(name)); attributes.push(o.literal(name));
if (attrI18nMetas.hasOwnProperty(name)) { if (attrI18nMetas.hasOwnProperty(name)) {
const meta = parseI18nMeta(attrI18nMetas[name]); const meta = parseI18nMeta(attrI18nMetas[name]);
const variable = this.constantPool.getTranslation(value, meta); const variable = this.constantPool.getTranslation(value, meta, this.fileBasedI18nSuffix);
attributes.push(variable); attributes.push(variable);
} else { } else {
attributes.push(o.literal(value)); attributes.push(o.literal(value));
@ -698,7 +705,8 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
// Create the template function // Create the template function
const templateVisitor = new TemplateDefinitionBuilder( const templateVisitor = new TemplateDefinitionBuilder(
this.constantPool, this._bindingScope, this.level + 1, contextName, templateName, [], this.constantPool, this._bindingScope, this.level + 1, contextName, templateName, [],
this.directiveMatcher, this.directives, this.pipeTypeByName, this.pipes, this._namespace); this.directiveMatcher, this.directives, this.pipeTypeByName, this.pipes, this._namespace,
this.fileBasedI18nSuffix);
// Nested templates must not be visited until after their parent templates have completed // Nested templates must not be visited until after their parent templates have completed
// processing, so they are queued here until after the initial pass. Otherwise, we wouldn't // processing, so they are queued here until after the initial pass. Otherwise, we wouldn't
@ -764,7 +772,7 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
// ``` // ```
visitSingleI18nTextChild(text: t.Text, i18nMeta: string) { visitSingleI18nTextChild(text: t.Text, i18nMeta: string) {
const meta = parseI18nMeta(i18nMeta); const meta = parseI18nMeta(i18nMeta);
const variable = this.constantPool.getTranslation(text.value, meta); const variable = this.constantPool.getTranslation(text.value, meta, this.fileBasedI18nSuffix);
this.creationInstruction( this.creationInstruction(
text.sourceSpan, R3.text, [o.literal(this.allocateDataSlot()), variable]); text.sourceSpan, R3.text, [o.literal(this.allocateDataSlot()), variable]);
} }
@ -1336,14 +1344,25 @@ function interpolate(args: o.Expression[]): o.Expression {
* @param templateUrl URL to use for source mapping of the parsed template * @param templateUrl URL to use for source mapping of the parsed template
*/ */
export function parseTemplate( export function parseTemplate(
template: string, templateUrl: string, options: {preserveWhitespaces?: boolean} = {}): template: string, templateUrl: string, options: {preserveWhitespaces?: boolean} = {},
{errors?: ParseError[], nodes: t.Node[], hasNgContent: boolean, ngContentSelectors: string[]} { relativeContextFilePath: string): {
errors?: ParseError[],
nodes: t.Node[],
hasNgContent: boolean,
ngContentSelectors: string[],
relativeContextFilePath: string
} {
const bindingParser = makeBindingParser(); const bindingParser = makeBindingParser();
const htmlParser = new HtmlParser(); const htmlParser = new HtmlParser();
const parseResult = htmlParser.parse(template, templateUrl); const parseResult = htmlParser.parse(template, templateUrl);
if (parseResult.errors && parseResult.errors.length > 0) { if (parseResult.errors && parseResult.errors.length > 0) {
return {errors: parseResult.errors, nodes: [], hasNgContent: false, ngContentSelectors: []}; return {
errors: parseResult.errors,
nodes: [],
hasNgContent: false,
ngContentSelectors: [], relativeContextFilePath
};
} }
let rootNodes: html.Node[] = parseResult.rootNodes; let rootNodes: html.Node[] = parseResult.rootNodes;
@ -1354,10 +1373,15 @@ export function parseTemplate(
const {nodes, hasNgContent, ngContentSelectors, errors} = const {nodes, hasNgContent, ngContentSelectors, errors} =
htmlAstToRender3Ast(rootNodes, bindingParser); htmlAstToRender3Ast(rootNodes, bindingParser);
if (errors && errors.length > 0) { if (errors && errors.length > 0) {
return {errors, nodes: [], hasNgContent: false, ngContentSelectors: []}; return {
errors,
nodes: [],
hasNgContent: false,
ngContentSelectors: [], relativeContextFilePath
};
} }
return {nodes, hasNgContent, ngContentSelectors}; return {nodes, hasNgContent, ngContentSelectors, relativeContextFilePath};
} }
/** /**

View File

@ -54,10 +54,11 @@ export function compileComponent(type: Type<any>, metadata: Component): void {
const constantPool = new ConstantPool(); const constantPool = new ConstantPool();
// Parse the template and check for errors. // Parse the template and check for errors.
const template = const template = parseTemplate(
parseTemplate(metadata.template !, `ng://${stringify(type)}/template.html`, { metadata.template !, `ng://${stringify(type)}/template.html`, {
preserveWhitespaces: metadata.preserveWhitespaces || false, preserveWhitespaces: metadata.preserveWhitespaces || false,
}); },
'');
if (template.errors !== undefined) { if (template.errors !== undefined) {
const errors = template.errors.map(err => err.toString()).join(', '); const errors = template.errors.map(err => err.toString()).join(', ');
throw new Error( throw new Error(