Skip to content

fix(@schematics/angular): improve i18n output path option migration #16412

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 11, 2019
Merged

fix(@schematics/angular): improve i18n output path option migration #16412

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 65 additions & 16 deletions packages/schematics/angular/migrations/update-9/update-workspace-config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@
* 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 { JsonAstObject } from '@angular-devkit/core';
import { JsonAstObject, logging } from '@angular-devkit/core';
import { Rule, Tree, UpdateRecorder } from '@angular-devkit/schematics';
import { posix } from 'path';
import { getWorkspacePath } from '../../utility/config';
import { NodeDependencyType, addPackageJsonDependency, getPackageJsonDependency } from '../../utility/dependencies';
import {
Expand All @@ -25,23 +26,23 @@ export const ANY_COMPONENT_STYLE_BUDGET = {
};

export function updateWorkspaceConfig(): Rule {
return (tree: Tree) => {
return (tree, context) => {
const workspacePath = getWorkspacePath(tree);
const workspace = getWorkspace(tree);
const recorder = tree.beginUpdate(workspacePath);

for (const { target, project } of getTargets(workspace, 'build', Builders.Browser)) {
for (const { target } of getTargets(workspace, 'build', Builders.Browser)) {
updateStyleOrScriptOption('styles', recorder, target);
updateStyleOrScriptOption('scripts', recorder, target);
addAnyComponentStyleBudget(recorder, target);
updateAotOption(tree, recorder, target);
addBuilderI18NOptions(recorder, target, project);
addBuilderI18NOptions(recorder, target, context.logger);
}

for (const { target, project } of getTargets(workspace, 'test', Builders.Karma)) {
for (const { target } of getTargets(workspace, 'test', Builders.Karma)) {
updateStyleOrScriptOption('styles', recorder, target);
updateStyleOrScriptOption('scripts', recorder, target);
addBuilderI18NOptions(recorder, target, project);
addBuilderI18NOptions(recorder, target, context.logger);
}

for (const { target } of getTargets(workspace, 'server', Builders.Server)) {
Expand Down Expand Up @@ -148,7 +149,11 @@ function addProjectI18NOptions(
}
}

function addBuilderI18NOptions(recorder: UpdateRecorder, builderConfig: JsonAstObject, projectConfig: JsonAstObject) {
function addBuilderI18NOptions(
recorder: UpdateRecorder,
builderConfig: JsonAstObject,
logger: logging.LoggerApi,
) {
const options = getAllOptions(builderConfig);
const mainOptions = findPropertyInAstObject(builderConfig, 'options');
const mainBaseHref =
Expand All @@ -160,31 +165,75 @@ function addBuilderI18NOptions(recorder: UpdateRecorder, builderConfig: JsonAstO

for (const option of options) {
const localeId = findPropertyInAstObject(option, 'i18nLocale');
const i18nFile = findPropertyInAstObject(option, 'i18nFile');

// The format is always auto-detected now
const i18nFormat = findPropertyInAstObject(option, 'i18nFormat');
if (i18nFormat) {
removePropertyInAstObject(recorder, option, 'i18nFormat');
}

const outputPath = findPropertyInAstObject(option, 'outputPath');
if (
localeId &&
localeId.kind === 'string' &&
i18nFile &&
outputPath &&
outputPath.kind === 'string'
) {
// This first block was intended to remove the redundant output path field
// but due to defects in the recorder, removing the option will cause malformed json
// if (
// mainOutputPathValue &&
// outputPath.value.match(
// new RegExp(`[/\\\\]?${mainOutputPathValue}[/\\\\]${localeId.value}[/\\\\]?$`),
// )
// ) {
// removePropertyInAstObject(recorder, option, 'outputPath');
// } else
if (outputPath.value.match(new RegExp(`[/\\\\]${localeId.value}[/\\\\]?$`))) {
const newOutputPath = outputPath.value.replace(
new RegExp(`[/\\\\]${localeId.value}[/\\\\]?$`),
'',
);
const { start, end } = outputPath;
recorder.remove(start.offset, end.offset - start.offset);
recorder.insertLeft(start.offset, `"${newOutputPath}"`);
} else {
logger.warn(
`Output path value "${outputPath.value}" for locale "${localeId.value}" is not supported with the new localization system. ` +
`With the current value, the localized output would be written to "${posix.join(
outputPath.value,
localeId.value,
)}". ` +
`Keeping existing options for the target configuration of locale "${localeId.value}".`,
);

continue;
}
}

if (localeId && localeId.kind === 'string') {
// add new localize option
insertPropertyInAstObjectInOrder(recorder, option, 'localize', [localeId.value], 12);
removePropertyInAstObject(recorder, option, 'i18nLocale');
}

const i18nFile = findPropertyInAstObject(option, 'i18nFile');
if (i18nFile) {
removePropertyInAstObject(recorder, option, 'i18nFile');
}

const i18nFormat = findPropertyInAstObject(option, 'i18nFormat');
if (i18nFormat) {
removePropertyInAstObject(recorder, option, 'i18nFormat');
}

// localize base HREF values are controlled by the i18n configuration
const baseHref = findPropertyInAstObject(option, 'baseHref');
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);
const { start, end } = baseHref;
recorder.remove(start.offset, end.offset - start.offset);
recorder.insertLeft(start.offset, `"/"`);
} else {
removePropertyInAstObject(recorder, option, 'baseHref');
}
}
}
Expand Down
34 changes: 33 additions & 1 deletion packages/schematics/angular/migrations/update-9/update-workspace-config_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ describe('Migration to version 9', () => {
describe('i18n configuration', () => {
function getI18NConfig(localId: string): object {
return {
outputPath: `dist/my-project-${localId}/`,
outputPath: `dist/my-project/${localId}/`,
i18nFile: `src/locale/messages.${localId}.xlf`,
i18nFormat: 'xlf',
i18nLocale: localId,
Expand Down Expand Up @@ -362,6 +362,38 @@ describe('Migration to version 9', () => {
expect(config.configurations.de.baseHref).toBeUndefined();
});

it('should remove outputPath option when used with i18n options and contains locale code', async () => {
let config = getWorkspaceTargets(tree);
config.build.options.outputPath = 'dist';
config.build.configurations.fr = { ...getI18NConfig('fr'), outputPath: 'dist/fr' };
config.build.configurations.de = { ...getI18NConfig('de'), outputPath: '/dist/de/' };
updateWorkspaceTargets(tree, config);

const tree2 = await schematicRunner.runSchematicAsync('workspace-version-9', {}, tree.branch()).toPromise();
config = getWorkspaceTargets(tree2).build;
expect(config.configurations.fr.outputPath).toBe('dist');
expect(config.configurations.de.outputPath).toBe('/dist');
});

it('should keep old i18n options except format when output path is not supported', async () => {
let config = getWorkspaceTargets(tree);
config.build.options.outputPath = 'dist';
config.build.configurations.fr = { ...getI18NConfig('fr'), outputPath: 'dist/abc' };
config.build.configurations.de = { ...getI18NConfig('de'), outputPath: '/dist/123' };
updateWorkspaceTargets(tree, config);

const tree2 = await schematicRunner.runSchematicAsync('workspace-version-9', {}, tree.branch()).toPromise();
config = getWorkspaceTargets(tree2).build;
expect(config.configurations.fr.outputPath).toBe('dist/abc');
expect(config.configurations.fr.i18nFormat).toBeUndefined();
expect(config.configurations.fr.i18nFile).toBeDefined();
expect(config.configurations.fr.i18nLocale).toBe('fr');
expect(config.configurations.de.outputPath).toBe('/dist/123');
expect(config.configurations.de.i18nFormat).toBeUndefined();
expect(config.configurations.de.i18nFile).toBeDefined();
expect(config.configurations.de.i18nLocale).toBe('de');
});

it('should keep baseHref option when not used with i18n options', async () => {
let config = getWorkspaceTargets(tree);
config.build.options = getI18NConfig('fr');
Expand Down