Skip to content

Commit

Permalink
fix(ivy): more descriptive errors for nested i18n sections (#33583)
Browse files Browse the repository at this point in the history
This commit moves nested i18n section detection to an earlier stage where we convert HTML AST to Ivy AST. This also gives a chance to produce better diagnistic message for nested i18n sections, that also includes a file name and location.

PR Close #33583
  • Loading branch information
AndrewKushnir authored and atscott committed Nov 5, 2019
1 parent 25aaff2 commit d9a3892
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 9 deletions.
Expand Up @@ -3415,14 +3415,57 @@ describe('i18n support in the template compiler', () => {
});

describe('errors', () => {
const verifyNestedSectionsError = (errorThrown: any, expectedErrorText: string) => {
expect(errorThrown.ngParseErrors.length).toBe(1);
const msg = errorThrown.ngParseErrors[0].toString();
expect(msg).toContain(
'Cannot mark an element as translatable inside of a translatable section. Please remove the nested i18n marker.');
expect(msg).toContain(expectedErrorText);
expect(msg).toMatch(/app\/spec\.ts\@\d+\:\d+/);
};

it('should throw on nested i18n sections', () => {
const files = getAppFilesWithTemplate(`
<div i18n><div i18n>Some content</div></div>
<div i18n>
<div i18n>Some content</div>
</div>
`);
try {
compile(files, angularFiles);
} catch (error) {
verifyNestedSectionsError(error, '[ERROR ->]<div i18n>Some content</div>');
}
});

it('should throw on nested i18n sections with tags in between', () => {
const files = getAppFilesWithTemplate(`
<div i18n>
<div>
<div i18n>Some content</div>
</div>
</div>
`);
expect(() => compile(files, angularFiles))
.toThrowError(
'Could not mark an element as translatable inside of a translatable section');
try {
compile(files, angularFiles);
} catch (error) {
verifyNestedSectionsError(error, '[ERROR ->]<div i18n>Some content</div>');
}
});

it('should throw on nested i18n sections represented with <ng-container>s', () => {
const files = getAppFilesWithTemplate(`
<ng-container i18n>
<div>
<ng-container i18n>Some content</ng-container>
</div>
</ng-container>
`);
try {
compile(files, angularFiles);
} catch (error) {
verifyNestedSectionsError(
error, '[ERROR ->]<ng-container i18n>Some content</ng-container>');
}
});
});
});
15 changes: 14 additions & 1 deletion packages/compiler/src/render3/r3_template_transform.ts
Expand Up @@ -80,11 +80,21 @@ class HtmlAstToIvyAst implements html.Visitor {
errors: ParseError[] = [];
styles: string[] = [];
styleUrls: string[] = [];
private inI18nBlock: boolean = false;

constructor(private bindingParser: BindingParser) {}

// HTML visitor
visitElement(element: html.Element): t.Node|null {
const isI18nRootElement = isI18nRootNode(element.i18n);
if (isI18nRootElement) {
if (this.inI18nBlock) {
this.reportError(
'Cannot mark an element as translatable inside of a translatable section. Please remove the nested i18n marker.',
element.sourceSpan);
}
this.inI18nBlock = true;
}
const preparsedElement = preparseElement(element);
if (preparsedElement.type === PreparsedElementType.SCRIPT) {
return null;
Expand Down Expand Up @@ -209,7 +219,7 @@ class HtmlAstToIvyAst implements html.Visitor {
// For <ng-template>s with structural directives on them, avoid passing i18n information to
// the wrapping template to prevent unnecessary i18n instructions from being generated. The
// necessary i18n meta information will be extracted from child elements.
const i18n = isTemplateElement && isI18nRootNode(element.i18n) ? undefined : element.i18n;
const i18n = isTemplateElement && isI18nRootElement ? undefined : element.i18n;

// TODO(pk): test for this case
parsedElement = new t.Template(
Expand All @@ -218,6 +228,9 @@ class HtmlAstToIvyAst implements html.Visitor {
templateVariables, element.sourceSpan, element.startSourceSpan, element.endSourceSpan,
i18n);
}
if (isI18nRootElement) {
this.inI18nBlock = false;
}
return parsedElement;
}

Expand Down
4 changes: 0 additions & 4 deletions packages/compiler/src/render3/view/template.ts
Expand Up @@ -535,10 +535,6 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
const isI18nRootElement: boolean =
isI18nRootNode(element.i18n) && !isSingleI18nIcu(element.i18n);

if (isI18nRootElement && this.i18n) {
throw new Error(`Could not mark an element as translatable inside of a translatable section`);
}

const i18nAttrs: (t.TextAttribute | t.BoundAttribute)[] = [];
const outputAttrs: t.TextAttribute[] = [];
let ngProjectAsAttr: t.TextAttribute|undefined;
Expand Down

0 comments on commit d9a3892

Please sign in to comment.