From 798a1240def1f1554918dfc4e74a9bc8e9bd8f62 Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Sun, 1 Dec 2019 19:50:06 -0500 Subject: [PATCH 1/3] fix(@angular-devkit/build-angular): augment base HREF when localizing All locale i18n options now support an object form which allows a base HREF to be defined for the locale. Each locale can now optionally define a custom base HREF that will be combined with the base HREF defined for the build configuration. By default if the shorthand form for the locale is used or the field is not present in the longhand form, the locale code will be used as the base HREF. To disable automatic augmentation a base HREF value of an empty string (`""`) can be used. This will prevent anything from being added to the existing base HREF. For common scenarios, the shorthand form will result in the preferred and recommended outcome of each built locale variant of the application containing a defined base HREF containing the locale code. --- packages/angular/cli/lib/config/schema.json | 50 ++++++++- .../build_angular/src/browser/index.ts | 14 ++- .../build_angular/src/utils/i18n-options.ts | 76 +++++++++---- .../workspace/workspace-schema.json | 50 ++++++++- .../e2e/tests/i18n/ivy-localize-basehref.ts | 103 ++++++++++++++++++ .../e2e/tests/i18n/ivy-localize-dl-xliff2.ts | 12 +- .../e2e/tests/i18n/ivy-localize-es2015.ts | 9 +- .../e2e/tests/i18n/ivy-localize-es5.ts | 9 +- tests/legacy-cli/e2e/tests/i18n/legacy.ts | 4 +- .../e2e/tests/i18n/ve-localize-es2015.ts | 9 +- 10 files changed, 296 insertions(+), 40 deletions(-) create mode 100644 tests/legacy-cli/e2e/tests/i18n/ivy-localize-basehref.ts diff --git a/packages/angular/cli/lib/config/schema.json b/packages/angular/cli/lib/config/schema.json index d2a5a6c3d0b8..cb1bfc431689 100644 --- a/packages/angular/cli/lib/config/schema.json +++ b/packages/angular/cli/lib/config/schema.json @@ -376,17 +376,57 @@ "type": "object", "properties": { "sourceLocale": { - "type": "string", - "description": "Specifies the source language of the application.", - "default": "en-US" + "oneOf": [ + { + "type": "string", + "description": "Specifies the source locale of the application.", + "default": "en-US", + "pattern": "^[a-z]{2}(-[a-zA-Z]{2,})?$" + }, + { + "type": "object", + "description": "Localization options to use for the source locale", + "properties": { + "code": { + "type": "string", + "description": "Specifies the locale code of the source locale", + "pattern": "^[a-z]{2}(-[a-zA-Z]{2,})?$" + }, + "baseHref": { + "type": "string", + "description": "HTML base HREF to use for the locale (defaults to the locale code)" + } + }, + "additionalProperties": false + } + ] }, "locales": { "type": "object", "additionalProperties": false, "patternProperties": { "^[a-z]{2}(-[a-zA-Z]{2,})?$": { - "type": "string", - "description": "Localization file to use for i18n" + "oneOf": [ + { + "type": "string", + "description": "Localization file to use for i18n" + }, + { + "type": "object", + "description": "Localization options to use for the locale", + "properties": { + "translation": { + "type": "string", + "description": "Localization file to use for i18n" + }, + "baseHref": { + "type": "string", + "description": "HTML base HREF to use for the locale (defaults to the locale code)" + } + }, + "additionalProperties": false + } + ] } } } diff --git a/packages/angular_devkit/build_angular/src/browser/index.ts b/packages/angular_devkit/build_angular/src/browser/index.ts index 30cd6a8020ae..eaf1dd66651c 100644 --- a/packages/angular_devkit/build_angular/src/browser/index.ts +++ b/packages/angular_devkit/build_angular/src/browser/index.ts @@ -673,6 +673,16 @@ export function buildWebpackBrowser( if (options.index) { for (const [locale, outputPath] of outputPaths.entries()) { + let localeBaseHref; + if (i18n.locales[locale] && i18n.locales[locale].baseHref !== '') { + localeBaseHref = path.posix.join( + options.baseHref || '', + i18n.locales[locale].baseHref === undefined + ? `/${locale}/` + : i18n.locales[locale].baseHref, + ); + } + try { await generateIndex( outputPath, @@ -684,6 +694,7 @@ export function buildWebpackBrowser( transforms.indexHtml, // i18nLocale is used when Ivy is disabled locale || options.i18nLocale, + localeBaseHref || options.baseHref, ); } catch (err) { return { success: false, error: mapErrorToMessage(err) }; @@ -734,6 +745,7 @@ function generateIndex( moduleFiles: EmittedFiles[] | undefined, transformer?: IndexHtmlTransform, locale?: string, + baseHref?: string, ): Promise { const host = new NodeJsSyncHost(); @@ -744,7 +756,7 @@ function generateIndex( files, noModuleFiles, moduleFiles, - baseHref: options.baseHref, + baseHref, deployUrl: options.deployUrl, sri: options.subresourceIntegrity, scripts: options.scripts, diff --git a/packages/angular_devkit/build_angular/src/utils/i18n-options.ts b/packages/angular_devkit/build_angular/src/utils/i18n-options.ts index ef7bed45c931..42a031e52787 100644 --- a/packages/angular_devkit/build_angular/src/utils/i18n-options.ts +++ b/packages/angular_devkit/build_angular/src/utils/i18n-options.ts @@ -21,7 +21,14 @@ export interface I18nOptions { sourceLocale: string; locales: Record< string, - { file: string; format?: string; translation?: unknown; dataPath?: string, integrity?: string } + { + file: string; + format?: string; + translation?: unknown; + dataPath?: string; + integrity?: string; + baseHref?: string; + } >; flatOutput?: boolean; readonly shouldInline: boolean; @@ -32,49 +39,79 @@ export function createI18nOptions( metadata: json.JsonObject, inline?: boolean | string[], ): I18nOptions { - if ( - metadata.i18n !== undefined && - (typeof metadata.i18n !== 'object' || !metadata.i18n || Array.isArray(metadata.i18n)) - ) { + if (metadata.i18n !== undefined && !json.isJsonObject(metadata.i18n)) { throw new Error('Project i18n field is malformed. Expected an object.'); } metadata = metadata.i18n || {}; - if (metadata.sourceLocale !== undefined && typeof metadata.sourceLocale !== 'string') { - throw new Error('Project i18n sourceLocale field is malformed. Expected a string.'); - } - const i18n: I18nOptions = { inlineLocales: new Set(), // en-US is the default locale added to Angular applications (https://angular.io/guide/i18n#i18n-pipes) - sourceLocale: metadata.sourceLocale || 'en-US', + sourceLocale: 'en-US', locales: {}, get shouldInline() { return this.inlineLocales.size > 0; }, }; - if ( - metadata.locales !== undefined && - (!metadata.locales || typeof metadata.locales !== 'object' || Array.isArray(metadata.locales)) - ) { + let rawSourceLocale; + let rawSourceLocaleBaseHref; + if (json.isJsonObject(metadata.sourceLocale)) { + rawSourceLocale = metadata.sourceLocale.code; + if (metadata.sourceLocale.baseHref !== undefined && typeof metadata.sourceLocale.baseHref !== 'string') { + throw new Error('Project i18n sourceLocale baseHref field is malformed. Expected a string.'); + } + rawSourceLocaleBaseHref = metadata.sourceLocale.baseHref; + } else { + rawSourceLocale = metadata.sourceLocale; + } + + if (rawSourceLocale !== undefined) { + if (typeof rawSourceLocale !== 'string') { + throw new Error('Project i18n sourceLocale field is malformed. Expected a string.'); + } + + i18n.sourceLocale = rawSourceLocale; + } + + i18n.locales[i18n.sourceLocale] = { + file: '', + baseHref: rawSourceLocaleBaseHref, + }; + + if (metadata.locales !== undefined && !json.isJsonObject(metadata.locales)) { throw new Error('Project i18n locales field is malformed. Expected an object.'); } else if (metadata.locales) { - for (const [locale, translationFile] of Object.entries(metadata.locales)) { - if (typeof translationFile !== 'string') { + for (const [locale, options] of Object.entries(metadata.locales)) { + let translationFile; + let baseHref; + if (json.isJsonObject(options)) { + if (typeof options.translation !== 'string') { + throw new Error( + `Project i18n locales translation field value for '${locale}' is malformed. Expected a string.`, + ); + } + translationFile = options.translation; + if (typeof options.baseHref === 'string') { + baseHref = options.baseHref; + } + } else if (typeof options !== 'string') { throw new Error( - `Project i18n locales field value for '${locale}' is malformed. Expected a string.`, + `Project i18n locales field value for '${locale}' is malformed. Expected a string or object.`, ); + } else { + translationFile = options; } if (locale === i18n.sourceLocale) { throw new Error( - `An i18n locale identifier ('${locale}') cannot both be a source locale and provide a translation.`, + `An i18n locale ('${locale}') cannot both be a source locale and provide a translation.`, ); } i18n.locales[locale] = { file: translationFile, + baseHref, }; } } @@ -252,11 +289,12 @@ function mergeDeprecatedI18nOptions( i18n.inlineLocales.add(i18nLocale); if (i18nFile !== undefined) { - i18n.locales[i18nLocale] = { file: i18nFile }; + i18n.locales[i18nLocale] = { file: i18nFile, baseHref: '' }; } else { // If no file, treat the locale as the source locale // This mimics deprecated behavior i18n.sourceLocale = i18nLocale; + i18n.locales[i18nLocale] = { file: '', baseHref: '' }; } i18n.flatOutput = true; diff --git a/packages/angular_devkit/core/src/experimental/workspace/workspace-schema.json b/packages/angular_devkit/core/src/experimental/workspace/workspace-schema.json index de7b5abec537..c03dca716f93 100644 --- a/packages/angular_devkit/core/src/experimental/workspace/workspace-schema.json +++ b/packages/angular_devkit/core/src/experimental/workspace/workspace-schema.json @@ -131,17 +131,57 @@ "type": "object", "properties": { "sourceLocale": { - "type": "string", - "description": "Specifies the source language of the application.", - "default": "en-US" + "oneOf": [ + { + "type": "string", + "description": "Specifies the source locale of the application.", + "default": "en-US", + "pattern": "^[a-z]{2}(-[a-zA-Z]{2,})?$" + }, + { + "type": "object", + "description": "Localization options to use for the source locale", + "properties": { + "code": { + "type": "string", + "description": "Specifies the locale code of the source locale", + "pattern": "^[a-z]{2}(-[a-zA-Z]{2,})?$" + }, + "baseHref": { + "type": "string", + "description": "HTML base HREF to use for the locale (defaults to the locale code)" + } + }, + "additionalProperties": false + } + ] }, "locales": { "type": "object", "additionalProperties": false, "patternProperties": { "^[a-z]{2}(-[a-zA-Z]{2,})?$": { - "type": "string", - "description": "Localization file to use for i18n." + "oneOf": [ + { + "type": "string", + "description": "Localization file to use for i18n" + }, + { + "type": "object", + "description": "Localization options to use for the locale", + "properties": { + "translation": { + "type": "string", + "description": "Localization file to use for i18n" + }, + "baseHref": { + "type": "string", + "description": "HTML base HREF to use for the locale (defaults to the locale code)" + } + }, + "additionalProperties": false + } + ] } } } diff --git a/tests/legacy-cli/e2e/tests/i18n/ivy-localize-basehref.ts b/tests/legacy-cli/e2e/tests/i18n/ivy-localize-basehref.ts new file mode 100644 index 000000000000..51ad75da4699 --- /dev/null +++ b/tests/legacy-cli/e2e/tests/i18n/ivy-localize-basehref.ts @@ -0,0 +1,103 @@ +/** + * @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 { expectFileToMatch } from '../../utils/fs'; +import { ng } from '../../utils/process'; +import { updateJsonFile } from '../../utils/project'; +import { externalServer, langTranslations, setupI18nConfig } from './legacy'; + +const baseHrefs = { + 'en-US': '/en/', + fr: '/fr-FR/', + de: '', +}; + +export default async function() { + // Setup i18n tests and config. + await setupI18nConfig(true); + + // Update angular.json + await updateJsonFile('angular.json', workspaceJson => { + const appProject = workspaceJson.projects['test-project']; + // tslint:disable-next-line: no-any + const i18n: Record = appProject.i18n; + + i18n.sourceLocale = { + baseHref: baseHrefs['en-US'], + }; + + i18n.locales['fr'] = { + translation: i18n.locales['fr'], + baseHref: baseHrefs['fr'], + }; + + i18n.locales['de'] = { + translation: i18n.locales['de'], + baseHref: baseHrefs['de'], + }; + }); + + // Build each locale and verify the output. + await ng('build'); + for (const { lang, outputPath } of langTranslations) { + if (baseHrefs[lang] === undefined) { + throw new Error('Invalid E2E test setup: unexpected locale ' + lang); + } + + // Verify the HTML lang attribute is present + await expectFileToMatch(`${outputPath}/index.html`, `lang="${lang}"`); + + // Verify the HTML base HREF attribute is present + await expectFileToMatch(`${outputPath}/index.html`, `href="${baseHrefs[lang] || '/'}"`); + + // Execute Application E2E tests with dev server + await ng('e2e', `--configuration=${lang}`, '--port=0'); + + // Execute Application E2E tests for a production build without dev server + const server = externalServer(outputPath, baseHrefs[lang] || '/'); + try { + await ng( + 'e2e', + `--configuration=${lang}`, + '--devServerTarget=', + `--baseUrl=http://localhost:4200${baseHrefs[lang] || '/'}`, + ); + } finally { + server.close(); + } + } + + // Update angular.json + await updateJsonFile('angular.json', workspaceJson => { + const appArchitect = workspaceJson.projects['test-project'].architect; + + appArchitect['build'].options.baseHref = '/test/'; + }); + + // Build each locale and verify the output. + await ng('build'); + for (const { lang, outputPath } of langTranslations) { + // Verify the HTML base HREF attribute is present + await expectFileToMatch(`${outputPath}/index.html`, `href="/test${baseHrefs[lang] || '/'}"`); + + // Execute Application E2E tests with dev server + await ng('e2e', `--configuration=${lang}`, '--port=0'); + + // Execute Application E2E tests for a production build without dev server + const server = externalServer(outputPath, '/test' + (baseHrefs[lang] || '/')); + try { + await ng( + 'e2e', + `--configuration=${lang}`, + '--devServerTarget=', + `--baseUrl=http://localhost:4200/test${baseHrefs[lang] || '/'}`, + ); + } finally { + server.close(); + } + } +} diff --git a/tests/legacy-cli/e2e/tests/i18n/ivy-localize-dl-xliff2.ts b/tests/legacy-cli/e2e/tests/i18n/ivy-localize-dl-xliff2.ts index cc45fdc6202b..28225d45a583 100644 --- a/tests/legacy-cli/e2e/tests/i18n/ivy-localize-dl-xliff2.ts +++ b/tests/legacy-cli/e2e/tests/i18n/ivy-localize-dl-xliff2.ts @@ -34,6 +34,9 @@ export async function executeTest() { // Verify the HTML lang attribute is present await expectFileToMatch(`${outputPath}/index.html`, `lang="${lang}"`); + // Verify the HTML base HREF attribute is present + await expectFileToMatch(`${outputPath}/index.html`, `href="/${lang}/"`); + // Verify the locale data is registered using the global files await expectFileToMatch(`${outputPath}/vendor-es5.js`, '.ng.common.locales'); await expectFileToMatch(`${outputPath}/vendor-es2015.js`, '.ng.common.locales'); @@ -42,9 +45,14 @@ export async function executeTest() { await ng('e2e', `--configuration=${lang}`, '--port=0'); // Execute Application E2E tests for a production build without dev server - const server = externalServer(outputPath); + const server = externalServer(outputPath, `/${lang}/`); try { - await ng('e2e', `--configuration=${lang}`, '--devServerTarget='); + await ng( + 'e2e', + `--configuration=${lang}`, + '--devServerTarget=', + `--baseUrl=http://localhost:4200/${lang}/`, + ); } finally { server.close(); } diff --git a/tests/legacy-cli/e2e/tests/i18n/ivy-localize-es2015.ts b/tests/legacy-cli/e2e/tests/i18n/ivy-localize-es2015.ts index 4e3901f4cd13..a0d9148277f4 100644 --- a/tests/legacy-cli/e2e/tests/i18n/ivy-localize-es2015.ts +++ b/tests/legacy-cli/e2e/tests/i18n/ivy-localize-es2015.ts @@ -29,9 +29,14 @@ export default async function() { await ng('e2e', `--configuration=${lang}`, '--port=0'); // Execute Application E2E tests for a production build without dev server - const server = externalServer(outputPath); + const server = externalServer(outputPath, `/${lang}/`); try { - await ng('e2e', `--configuration=${lang}`, '--devServerTarget='); + await ng( + 'e2e', + `--configuration=${lang}`, + '--devServerTarget=', + `--baseUrl=http://localhost:4200/${lang}/`, + ); } finally { server.close(); } diff --git a/tests/legacy-cli/e2e/tests/i18n/ivy-localize-es5.ts b/tests/legacy-cli/e2e/tests/i18n/ivy-localize-es5.ts index eee1e7251827..656db6ec58f6 100644 --- a/tests/legacy-cli/e2e/tests/i18n/ivy-localize-es5.ts +++ b/tests/legacy-cli/e2e/tests/i18n/ivy-localize-es5.ts @@ -29,9 +29,14 @@ export default async function() { await ng('e2e', `--configuration=${lang}`, '--port=0'); // Execute Application E2E tests for a production build without dev server - const server = externalServer(outputPath); + const server = externalServer(outputPath, `/${lang}/`); try { - await ng('e2e', `--configuration=${lang}`, '--devServerTarget='); + await ng( + 'e2e', + `--configuration=${lang}`, + '--devServerTarget=', + `--baseUrl=http://localhost:4200/${lang}/`, + ); } finally { server.close(); } diff --git a/tests/legacy-cli/e2e/tests/i18n/legacy.ts b/tests/legacy-cli/e2e/tests/i18n/legacy.ts index ff02cbba1b0c..d40d900421c4 100644 --- a/tests/legacy-cli/e2e/tests/i18n/legacy.ts +++ b/tests/legacy-cli/e2e/tests/i18n/legacy.ts @@ -56,9 +56,9 @@ export const langTranslations = [ ]; export const sourceLocale = langTranslations[0].lang; -export const externalServer = (outputPath: string) => { +export const externalServer = (outputPath: string, baseUrl = '/') => { const app = express(); - app.use(express.static(resolve(outputPath))); + app.use(baseUrl, express.static(resolve(outputPath))); // call .close() on the return value to close the server. return app.listen(4200, 'localhost'); diff --git a/tests/legacy-cli/e2e/tests/i18n/ve-localize-es2015.ts b/tests/legacy-cli/e2e/tests/i18n/ve-localize-es2015.ts index c02c411454b9..91194763444a 100644 --- a/tests/legacy-cli/e2e/tests/i18n/ve-localize-es2015.ts +++ b/tests/legacy-cli/e2e/tests/i18n/ve-localize-es2015.ts @@ -44,9 +44,14 @@ export default async function() { await ng('e2e', `--configuration=${lang}`, '--port=0'); // Execute Application E2E tests for a production build without dev server - const server = externalServer(outputPath); + const server = externalServer(outputPath, `/${lang}/`); try { - await ng('e2e', `--configuration=${lang}`, '--devServerTarget='); + await ng( + 'e2e', + `--configuration=${lang}`, + '--devServerTarget=', + `--baseUrl=http://localhost:4200/${lang}/`, + ); } finally { server.close(); } From 8b4aa9a9076173b15ea92ca930b5d818585deec4 Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Mon, 2 Dec 2019 15:35:43 -0500 Subject: [PATCH 2/3] fix(@schematics/angular): migrate localized base HREF options for 9.0 --- .../update-9/update-workspace-config.ts | 54 +++++++++++++- .../update-9/update-workspace-config_spec.ts | 72 ++++++++++++++++++- 2 files changed, 120 insertions(+), 6 deletions(-) diff --git a/packages/schematics/angular/migrations/update-9/update-workspace-config.ts b/packages/schematics/angular/migrations/update-9/update-workspace-config.ts index bfcfa20fdf91..890301c2939b 100644 --- a/packages/schematics/angular/migrations/update-9/update-workspace-config.ts +++ b/packages/schematics/angular/migrations/update-9/update-workspace-config.ts @@ -70,8 +70,16 @@ function addProjectI18NOptions( return; } + const mainOptions = findPropertyInAstObject(browserConfig, 'options'); + const mainBaseHref = + mainOptions && + mainOptions.kind === 'object' && + findPropertyInAstObject(mainOptions, 'baseHref'); + const hasMainBaseHref = + !!mainBaseHref && mainBaseHref.kind === 'string' && mainBaseHref.value !== '/'; + // browser builder options - let locales: Record | undefined; + let locales: Record | undefined; const options = getAllOptions(browserConfig); for (const option of options) { const localeId = findPropertyInAstObject(option, 'i18nLocale'); @@ -87,12 +95,37 @@ function addProjectI18NOptions( const localIdValue = localeId.value; const localeFileValue = localeFile.value; + const baseHref = findPropertyInAstObject(option, 'baseHref'); + let baseHrefValue; + // if the main options has a non-default base href, the i18n configuration + // for the locale baseHref is disabled to more obviously mimic existing behavior + if (baseHref && !hasMainBaseHref) { + if (baseHref.kind === 'string' && baseHref.value !== `/${localIdValue}/`) { + baseHrefValue = baseHref.value; + } + } else { + // If the configuration does not contain a baseHref, ensure the main option value is used. + baseHrefValue = ''; + } + if (!locales) { locales = { - [localIdValue]: localeFileValue, + [localIdValue]: + baseHrefValue === undefined + ? localeFileValue + : { + translation: localeFileValue, + baseHref: baseHrefValue, + }, }; } else { - locales[localIdValue] = localeFileValue; + locales[localIdValue] = + baseHrefValue === undefined + ? localeFileValue + : { + translation: localeFileValue, + baseHref: baseHrefValue, + }; } } @@ -127,6 +160,13 @@ function addProjectI18NOptions( function addBuilderI18NOptions(recorder: UpdateRecorder, builderConfig: JsonAstObject, projectConfig: JsonAstObject) { const options = getAllOptions(builderConfig); + const mainOptions = findPropertyInAstObject(builderConfig, 'options'); + const mainBaseHref = + mainOptions && + mainOptions.kind === 'object' && + findPropertyInAstObject(mainOptions, 'baseHref'); + const hasMainBaseHref = + !!mainBaseHref && mainBaseHref.kind === 'string' && mainBaseHref.value !== '/'; for (const option of options) { const localeId = findPropertyInAstObject(option, 'i18nLocale'); @@ -145,6 +185,14 @@ function addBuilderI18NOptions(recorder: UpdateRecorder, builderConfig: JsonAstO if (i18nFormat) { removePropertyInAstObject(recorder, option, 'i18nFormat'); } + + // localize base HREF values are controlled by the i18n configuration + // except if the main options has a non-default base href, the i18n configuration + // for the locale baseHref is disabled in that case to more obviously mimic existing behavior + const baseHref = findPropertyInAstObject(option, 'baseHref'); + if (localeId && i18nFile && baseHref && !hasMainBaseHref) { + removePropertyInAstObject(recorder, option, 'baseHref'); + } } } diff --git a/packages/schematics/angular/migrations/update-9/update-workspace-config_spec.ts b/packages/schematics/angular/migrations/update-9/update-workspace-config_spec.ts index 7c2ed8b38cb6..29b6674f1938 100644 --- a/packages/schematics/angular/migrations/update-9/update-workspace-config_spec.ts +++ b/packages/schematics/angular/migrations/update-9/update-workspace-config_spec.ts @@ -350,6 +350,42 @@ describe('Migration to version 9', () => { expect(config.configurations.de.i18nLocale).toBeUndefined(); }); + it('should remove baseHref option when used with i18n options and no main base href', async () => { + let config = getWorkspaceTargets(tree); + config.build.configurations.fr = { ...getI18NConfig('fr'), baseHref: '/fr/' }; + config.build.configurations.de = { ...getI18NConfig('de'), baseHref: '/abc/' }; + updateWorkspaceTargets(tree, config); + + const tree2 = await schematicRunner.runSchematicAsync('workspace-version-9', {}, tree.branch()).toPromise(); + config = getWorkspaceTargets(tree2).build; + expect(config.configurations.fr.baseHref).toBeUndefined(); + expect(config.configurations.de.baseHref).toBeUndefined(); + }); + + it('should keep baseHref option when not used with i18n options', async () => { + let config = getWorkspaceTargets(tree); + config.build.options = getI18NConfig('fr'); + config.build.configurations.de = getI18NConfig('de'); + config.build.configurations.staging = { baseHref: '/de/' }; + updateWorkspaceTargets(tree, config); + + const tree2 = await schematicRunner.runSchematicAsync('workspace-version-9', {}, tree.branch()).toPromise(); + config = getWorkspaceTargets(tree2).build; + expect(config.configurations.staging.baseHref).toBe('/de/'); + }); + + it('should keep baseHref options when used with i18n options and main baseHref option', async () => { + let config = getWorkspaceTargets(tree); + config.build.options = { baseHref: '/my-app/' }; + config.build.configurations.de = { ...getI18NConfig('de'), baseHref: '/de/' }; + updateWorkspaceTargets(tree, config); + + const tree2 = await schematicRunner.runSchematicAsync('workspace-version-9', {}, tree.branch()).toPromise(); + config = getWorkspaceTargets(tree2).build; + expect(config.options.baseHref).toBe('/my-app/'); + expect(config.configurations.de.baseHref).toBe('/de/'); + }); + it('should remove deprecated extract-i18n options', async () => { let config = getWorkspaceTargets(tree); config['extract-i18n'].options.i18nFormat = 'xmb'; @@ -379,8 +415,7 @@ describe('Migration to version 9', () => { it(`should add i18n 'locales' project config`, async () => { const config = getWorkspaceTargets(tree); - config.build.options.aot = false; - config.build.options = getI18NConfig('fr'); + config.build.configurations.fr = getI18NConfig('fr'); config.build.configurations.de = getI18NConfig('de'); updateWorkspaceTargets(tree, config); @@ -388,10 +423,41 @@ describe('Migration to version 9', () => { const projectConfig = JSON.parse(tree2.readContent(workspacePath)).projects['migration-test']; expect(projectConfig.i18n.sourceLocale).toBeUndefined(); expect(projectConfig.i18n.locales).toEqual({ - de: 'src/locale/messages.de.xlf', + de: { translation: 'src/locale/messages.de.xlf', baseHref: '' }, + fr: { translation: 'src/locale/messages.fr.xlf', baseHref: '' }, + }); + }); + + it(`should add i18n 'locales' project config when using baseHref options`, async () => { + const config = getWorkspaceTargets(tree); + config.build.configurations.fr = { ...getI18NConfig('fr'), baseHref: '/fr/' }; + config.build.configurations.de = { ...getI18NConfig('de'), baseHref: '/abc/' }; + updateWorkspaceTargets(tree, config); + + const tree2 = await schematicRunner.runSchematicAsync('workspace-version-9', {}, tree.branch()).toPromise(); + const projectConfig = JSON.parse(tree2.readContent(workspacePath)).projects['migration-test']; + expect(projectConfig.i18n.sourceLocale).toBeUndefined(); + expect(projectConfig.i18n.locales).toEqual({ + de: { translation: 'src/locale/messages.de.xlf', baseHref: '/abc/' }, fr: 'src/locale/messages.fr.xlf', }); }); + + it(`should add i18n 'locales' project config when using baseHref options and main base href`, async () => { + const config = getWorkspaceTargets(tree); + config.build.options = { baseHref: '/my-app/' }; + config.build.configurations.fr = { ...getI18NConfig('fr'), baseHref: '/fr/' }; + config.build.configurations.de = { ...getI18NConfig('de'), baseHref: '/abc/' }; + updateWorkspaceTargets(tree, config); + + const tree2 = await schematicRunner.runSchematicAsync('workspace-version-9', {}, tree.branch()).toPromise(); + const projectConfig = JSON.parse(tree2.readContent(workspacePath)).projects['migration-test']; + expect(projectConfig.i18n.sourceLocale).toBeUndefined(); + expect(projectConfig.i18n.locales).toEqual({ + de: { translation: 'src/locale/messages.de.xlf', baseHref: '' }, + fr: { translation: 'src/locale/messages.fr.xlf', baseHref: '' }, + }); + }); }); describe('when i18n builder options are not set', () => { From 33ed6b1e80dc60efddbf8b48809ebdb2fdf6ccca Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Tue, 3 Dec 2019 20:33:06 -0500 Subject: [PATCH 3/3] fix(@schematics/angular): improve i18n baseHref migration support for direct localize usage --- .../update-9/update-workspace-config.ts | 22 +++++++------------ .../update-9/update-workspace-config_spec.ts | 6 ++--- 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/packages/schematics/angular/migrations/update-9/update-workspace-config.ts b/packages/schematics/angular/migrations/update-9/update-workspace-config.ts index 890301c2939b..f038f8d58a1d 100644 --- a/packages/schematics/angular/migrations/update-9/update-workspace-config.ts +++ b/packages/schematics/angular/migrations/update-9/update-workspace-config.ts @@ -70,14 +70,6 @@ function addProjectI18NOptions( return; } - const mainOptions = findPropertyInAstObject(browserConfig, 'options'); - const mainBaseHref = - mainOptions && - mainOptions.kind === 'object' && - findPropertyInAstObject(mainOptions, 'baseHref'); - const hasMainBaseHref = - !!mainBaseHref && mainBaseHref.kind === 'string' && mainBaseHref.value !== '/'; - // browser builder options let locales: Record | undefined; const options = getAllOptions(browserConfig); @@ -97,9 +89,7 @@ function addProjectI18NOptions( const baseHref = findPropertyInAstObject(option, 'baseHref'); let baseHrefValue; - // if the main options has a non-default base href, the i18n configuration - // for the locale baseHref is disabled to more obviously mimic existing behavior - if (baseHref && !hasMainBaseHref) { + if (baseHref) { if (baseHref.kind === 'string' && baseHref.value !== `/${localIdValue}/`) { baseHrefValue = baseHref.value; } @@ -187,11 +177,15 @@ function addBuilderI18NOptions(recorder: UpdateRecorder, builderConfig: JsonAstO } // localize base HREF values are controlled by the i18n configuration - // except if the main options has a non-default base href, the i18n configuration - // for the locale baseHref is disabled in that case to more obviously mimic existing behavior const baseHref = findPropertyInAstObject(option, 'baseHref'); - if (localeId && i18nFile && baseHref && !hasMainBaseHref) { + if (localeId && i18nFile && baseHref) { removePropertyInAstObject(recorder, option, 'baseHref'); + + // if the main option set has a non-default base href, + // ensure that the augmented base href has the correct base value + if (hasMainBaseHref) { + insertPropertyInAstObjectInOrder(recorder, option, 'baseHref', '/', 12); + } } } } diff --git a/packages/schematics/angular/migrations/update-9/update-workspace-config_spec.ts b/packages/schematics/angular/migrations/update-9/update-workspace-config_spec.ts index 29b6674f1938..611c0459c9e2 100644 --- a/packages/schematics/angular/migrations/update-9/update-workspace-config_spec.ts +++ b/packages/schematics/angular/migrations/update-9/update-workspace-config_spec.ts @@ -383,7 +383,7 @@ describe('Migration to version 9', () => { const tree2 = await schematicRunner.runSchematicAsync('workspace-version-9', {}, tree.branch()).toPromise(); config = getWorkspaceTargets(tree2).build; expect(config.options.baseHref).toBe('/my-app/'); - expect(config.configurations.de.baseHref).toBe('/de/'); + expect(config.configurations.de.baseHref).toBe('/'); }); it('should remove deprecated extract-i18n options', async () => { @@ -454,8 +454,8 @@ describe('Migration to version 9', () => { const projectConfig = JSON.parse(tree2.readContent(workspacePath)).projects['migration-test']; expect(projectConfig.i18n.sourceLocale).toBeUndefined(); expect(projectConfig.i18n.locales).toEqual({ - de: { translation: 'src/locale/messages.de.xlf', baseHref: '' }, - fr: { translation: 'src/locale/messages.fr.xlf', baseHref: '' }, + de: { translation: 'src/locale/messages.de.xlf', baseHref: '/abc/' }, + fr: 'src/locale/messages.fr.xlf', }); }); });