From 9c3386b1b771ffcd9d88f6768deb8fb2641d95af Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Fri, 7 Jul 2017 16:16:49 -0700 Subject: [PATCH] fix(compiler): remove i18n markup even if no translations (#17999) Fixes #11042 --- .../integrationtest/alt/src/bootstrap.ts | 12 +++++ .../integrationtest/test/basic_spec.ts | 13 ++++++ .../integrationtest/test/util_alt.ts | 33 ++++++++++++++ .../integrationtest/tsconfig-build-alt.json | 32 ++++++++++++++ packages/compiler-cli/src/codegen.ts | 3 ++ packages/compiler/src/i18n/extractor.ts | 4 +- .../compiler/src/i18n/i18n_html_parser.ts | 11 ++--- packages/compiler/src/jit/compiler_factory.ts | 11 +++-- .../test/i18n/extractor_merger_spec.ts | 44 +++++++++++++++++-- .../test/i18n/i18n_html_parser_spec.ts | 16 ------- scripts/ci/offline_compiler_test.sh | 1 + 11 files changed, 149 insertions(+), 31 deletions(-) create mode 100644 packages/compiler-cli/integrationtest/alt/src/bootstrap.ts create mode 100644 packages/compiler-cli/integrationtest/test/util_alt.ts create mode 100644 packages/compiler-cli/integrationtest/tsconfig-build-alt.json diff --git a/packages/compiler-cli/integrationtest/alt/src/bootstrap.ts b/packages/compiler-cli/integrationtest/alt/src/bootstrap.ts new file mode 100644 index 0000000000..d6d7aedc7f --- /dev/null +++ b/packages/compiler-cli/integrationtest/alt/src/bootstrap.ts @@ -0,0 +1,12 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {BasicComp} from '../../src/basic'; +import {MainModuleNgFactory} from './module.ngfactory'; + +MainModuleNgFactory.create(null).instance.appRef.bootstrap(BasicComp); diff --git a/packages/compiler-cli/integrationtest/test/basic_spec.ts b/packages/compiler-cli/integrationtest/test/basic_spec.ts index ff57690237..e48e60074d 100644 --- a/packages/compiler-cli/integrationtest/test/basic_spec.ts +++ b/packages/compiler-cli/integrationtest/test/basic_spec.ts @@ -12,6 +12,7 @@ import * as path from 'path'; import {MultipleComponentsMyComp} from '../src/a/multiple_components'; import {BasicComp} from '../src/basic'; import {createComponent} from './util'; +import {createComponentAlt} from './util_alt'; describe('template codegen output', () => { const outDir = 'src'; @@ -88,5 +89,17 @@ describe('template codegen output', () => { const pText = pElement.children.map((c: any) => c.data).join('').trim(); expect(pText).toBe('tervetuloa'); }); + + it('should have removed i18n markup', () => { + const containerElement = createComponent(BasicComp).debugElement.children[0]; + expect(containerElement.attributes['title']).toBe('käännä teksti'); + expect(containerElement.attributes['i18n-title']).toBeUndefined(); + }); + + it('should have removed i18n markup event without translations', () => { + const containerElement = createComponentAlt(BasicComp).debugElement.children[0]; + expect(containerElement.attributes['title']).toBe('translate me'); + expect(containerElement.attributes['i18n-title']).toBeUndefined(); + }); }); }); diff --git a/packages/compiler-cli/integrationtest/test/util_alt.ts b/packages/compiler-cli/integrationtest/test/util_alt.ts new file mode 100644 index 0000000000..972dc03ac9 --- /dev/null +++ b/packages/compiler-cli/integrationtest/test/util_alt.ts @@ -0,0 +1,33 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {NgModuleRef} from '@angular/core'; +import {ComponentFixture} from '@angular/core/testing'; +import {platformServerTesting} from '@angular/platform-server/testing'; + +import {MainModuleNgFactory} from '../alt/src/module.ngfactory'; +import {MainModule} from '../src/module'; + +let mainModuleRef: NgModuleRef = null !; +beforeEach((done) => { + platformServerTesting().bootstrapModuleFactory(MainModuleNgFactory).then((moduleRef: any) => { + mainModuleRef = moduleRef; + done(); + }); +}); + +export function createModule(): NgModuleRef { + return mainModuleRef; +} + +export function createComponentAlt(comp: {new (...args: any[]): C}): ComponentFixture { + const moduleRef = createModule(); + const compRef = + moduleRef.componentFactoryResolver.resolveComponentFactory(comp).create(moduleRef.injector); + return new ComponentFixture(compRef, null, false); +} diff --git a/packages/compiler-cli/integrationtest/tsconfig-build-alt.json b/packages/compiler-cli/integrationtest/tsconfig-build-alt.json new file mode 100644 index 0000000000..2a1553a619 --- /dev/null +++ b/packages/compiler-cli/integrationtest/tsconfig-build-alt.json @@ -0,0 +1,32 @@ +{ + "angularCompilerOptions": { + // For TypeScript 1.8, we have to lay out generated files + // in the same source directory with your code. + "genDir": "./alt", + "debug": true, + "enableSummariesForJit": true, + "alwaysCompileGeneratedCode": true + }, + + "compilerOptions": { + "target": "es5", + "experimentalDecorators": true, + "noImplicitAny": true, + "strictNullChecks": true, + "skipLibCheck": true, + "moduleResolution": "node", + "rootDir": "", + "declaration": true, + "lib": ["es6", "dom"], + "baseUrl": ".", + // Prevent scanning up the directory tree for types + "typeRoots": ["node_modules/@types"], + "noUnusedLocals": true, + "sourceMap": true + }, + + "files": [ + "src/module", + "alt/src/bootstrap" + ] +} diff --git a/packages/compiler-cli/src/codegen.ts b/packages/compiler-cli/src/codegen.ts index 675e6fbb3d..8207c74e2e 100644 --- a/packages/compiler-cli/src/codegen.ts +++ b/packages/compiler-cli/src/codegen.ts @@ -95,6 +95,9 @@ export class CodeGenerator { `Unknown option for missingTranslation (${cliOptions.missingTranslation}). Use either error, warning or ignore.`); } } + if (!transContent) { + missingTranslation = MissingTranslationStrategy.Ignore + } const {compiler: aotCompiler} = compiler.createAotCompiler(ngCompilerHost, { translations: transContent, i18nFormat: cliOptions.i18nFormat, diff --git a/packages/compiler/src/i18n/extractor.ts b/packages/compiler/src/i18n/extractor.ts index 9af0ac30d4..5d6cbb5d1f 100644 --- a/packages/compiler/src/i18n/extractor.ts +++ b/packages/compiler/src/i18n/extractor.ts @@ -29,8 +29,6 @@ import {ParseError} from '../parse_util'; import {PipeResolver} from '../pipe_resolver'; import {DomElementSchemaRegistry} from '../schema/dom_element_schema_registry'; import {createOfflineCompileUrlResolver} from '../url_resolver'; - -import {I18NHtmlParser} from './i18n_html_parser'; import {MessageBundle} from './message_bundle'; /** @@ -87,7 +85,7 @@ export class Extractor { static create(host: ExtractorHost, locale: string|null): {extractor: Extractor, staticReflector: StaticReflector} { - const htmlParser = new I18NHtmlParser(new HtmlParser()); + const htmlParser = new HtmlParser(); const urlResolver = createOfflineCompileUrlResolver(); const symbolCache = new StaticSymbolCache(); diff --git a/packages/compiler/src/i18n/i18n_html_parser.ts b/packages/compiler/src/i18n/i18n_html_parser.ts index 7a05ba42ce..aab0aa0b95 100644 --- a/packages/compiler/src/i18n/i18n_html_parser.ts +++ b/packages/compiler/src/i18n/i18n_html_parser.ts @@ -7,9 +7,12 @@ */ import {MissingTranslationStrategy, ɵConsole as Console} from '@angular/core'; + import {HtmlParser} from '../ml_parser/html_parser'; import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from '../ml_parser/interpolation_config'; import {ParseTreeResult} from '../ml_parser/parser'; + +import {digest} from './digest'; import {mergeTranslations} from './extractor_merger'; import {Serializer} from './serializers/serializer'; import {Xliff} from './serializers/xliff'; @@ -32,6 +35,9 @@ export class I18NHtmlParser implements HtmlParser { const serializer = createSerializer(translationsFormat); this._translationBundle = TranslationBundle.load(translations, 'i18n', serializer, missingTranslation, console); + } else { + this._translationBundle = + new TranslationBundle({}, null, digest, undefined, missingTranslation, console); } } @@ -41,11 +47,6 @@ export class I18NHtmlParser implements HtmlParser { const parseResult = this._htmlParser.parse(source, url, parseExpansionForms, interpolationConfig); - if (!this._translationBundle) { - // Do not enable i18n when no translation bundle is provided - return parseResult; - } - if (parseResult.errors.length) { return new ParseTreeResult(parseResult.rootNodes, parseResult.errors); } diff --git a/packages/compiler/src/jit/compiler_factory.ts b/packages/compiler/src/jit/compiler_factory.ts index c8b9b59aec..fc2458ea0d 100644 --- a/packages/compiler/src/jit/compiler_factory.ts +++ b/packages/compiler/src/jit/compiler_factory.ts @@ -59,10 +59,13 @@ export const COMPILER_PROVIDERS: Array|{[k: string]: any}|any[]> = }, { provide: i18n.I18NHtmlParser, - useFactory: (parser: HtmlParser, translations: string, format: string, config: CompilerConfig, - console: Console) => - new i18n.I18NHtmlParser( - parser, translations, format, config.missingTranslation !, console), + useFactory: (parser: HtmlParser, translations: string | null, format: string, + config: CompilerConfig, console: Console) => { + translations = translations || ''; + const missingTranslation = + translations ? config.missingTranslation ! : MissingTranslationStrategy.Ignore; + return new i18n.I18NHtmlParser(parser, translations, format, missingTranslation, console); + }, deps: [ baseHtmlParser, [new Optional(), new Inject(TRANSLATIONS)], diff --git a/packages/compiler/test/i18n/extractor_merger_spec.ts b/packages/compiler/test/i18n/extractor_merger_spec.ts index 7ba9ea413f..641b769c3f 100644 --- a/packages/compiler/test/i18n/extractor_merger_spec.ts +++ b/packages/compiler/test/i18n/extractor_merger_spec.ts @@ -7,6 +7,7 @@ */ import {DEFAULT_INTERPOLATION_CONFIG, HtmlParser} from '@angular/compiler'; +import {MissingTranslationStrategy} from '@angular/core'; import {digest, serializeNodes as serializeI18nNodes} from '../../src/i18n/digest'; import {extractMessages, mergeTranslations} from '../../src/i18n/extractor_merger'; @@ -465,6 +466,31 @@ export function main() { .toEqual(`
some element
`); }); }); + + describe('no translations', () => { + it('should remove i18n attributes', () => { + const HTML = `

foo

`; + expect(fakeNoTranslate(HTML)).toEqual('

foo

'); + }); + + it('should remove i18n- attributes', () => { + const HTML = `

`; + expect(fakeNoTranslate(HTML)).toEqual('

'); + }); + + it('should remove i18n comment blocks', () => { + const HTML = `before

foo

barafter`; + expect(fakeNoTranslate(HTML)).toEqual('before

foo

barafter'); + }); + + it('should remove nested i18n markup', () => { + const HTML = + `foo
{count, plural, =0 {

}}
`; + expect(fakeNoTranslate(HTML)) + .toEqual( + 'foo
{count, plural, =0 {

}}
'); + }); + }) }); } @@ -493,10 +519,22 @@ function fakeTranslate( i18nMsgMap[id] = [new i18n.Text(`**${text}**`, null !)]; }); - const translations = new TranslationBundle(i18nMsgMap, null, digest); - + const translationBundle = new TranslationBundle(i18nMsgMap, null, digest); const output = mergeTranslations( - htmlNodes, translations, DEFAULT_INTERPOLATION_CONFIG, implicitTags, implicitAttrs); + htmlNodes, translationBundle, DEFAULT_INTERPOLATION_CONFIG, implicitTags, implicitAttrs); + expect(output.errors).toEqual([]); + + return serializeHtmlNodes(output.rootNodes).join(''); +} + +function fakeNoTranslate( + content: string, implicitTags: string[] = [], + implicitAttrs: {[k: string]: string[]} = {}): string { + const htmlNodes: html.Node[] = parseHtml(content); + const translationBundle = new TranslationBundle( + {}, null, digest, undefined, MissingTranslationStrategy.Ignore, console); + const output = mergeTranslations( + htmlNodes, translationBundle, DEFAULT_INTERPOLATION_CONFIG, implicitTags, implicitAttrs); expect(output.errors).toEqual([]); return serializeHtmlNodes(output.rootNodes).join(''); diff --git a/packages/compiler/test/i18n/i18n_html_parser_spec.ts b/packages/compiler/test/i18n/i18n_html_parser_spec.ts index aec56dfa13..84afbd9e7d 100644 --- a/packages/compiler/test/i18n/i18n_html_parser_spec.ts +++ b/packages/compiler/test/i18n/i18n_html_parser_spec.ts @@ -13,22 +13,6 @@ import {ParseTreeResult} from '@angular/compiler/src/ml_parser/parser'; export function main() { describe('I18N html parser', () => { - - it('should return the html nodes when no translations are given', () => { - const htmlParser = new HtmlParser(); - const i18nHtmlParser = new I18NHtmlParser(htmlParser); - const ptResult = new ParseTreeResult([], []); - - spyOn(htmlParser, 'parse').and.returnValue(ptResult); - spyOn(i18nHtmlParser, 'parse').and.callThrough(); - - expect(i18nHtmlParser.parse('source', 'url')).toBe(ptResult); - - expect(htmlParser.parse).toHaveBeenCalledTimes(1); - expect(htmlParser.parse) - .toHaveBeenCalledWith('source', 'url', jasmine.anything(), jasmine.anything()); - }); - // https://github.com/angular/angular/issues/14322 it('should parse the translations only once', () => { const transBundle = new TranslationBundle({}, null, () => 'id'); diff --git a/scripts/ci/offline_compiler_test.sh b/scripts/ci/offline_compiler_test.sh index 665fa405fc..a938ca911b 100755 --- a/scripts/ci/offline_compiler_test.sh +++ b/scripts/ci/offline_compiler_test.sh @@ -55,6 +55,7 @@ cp -v package.json $TMP # Copy the html files from source to the emitted output cp flat_module/src/*.html node_modules/flat_module/src + ./node_modules/.bin/ngc -p tsconfig-build-alt.json --missingTranslation=error --i18nFormat=xlf ./node_modules/.bin/ngc -p tsconfig-build.json --i18nFile=src/messages.fi.xlf --locale=fi --i18nFormat=xlf ./node_modules/.bin/ng-xi18n -p tsconfig-xi18n.json --i18nFormat=xlf --locale=fr