Skip to content
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.

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

fix(@angular-devkit/build-angular): allow localization with development server #16053

Merged
merged 2 commits into from Nov 8, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/angular_devkit/build_angular/package.json
Expand Up @@ -17,6 +17,7 @@
"@ngtools/webpack": "0.0.0",
"ajv": "6.10.2",
"autoprefixer": "9.7.1",
"babel-loader": "8.0.6",
"browserslist": "4.7.2",
"cacache": "13.0.1",
"caniuse-lite": "1.0.30001008",
Expand Down
2 changes: 1 addition & 1 deletion packages/angular_devkit/build_angular/src/browser/index.ts
Expand Up @@ -355,7 +355,7 @@ export function buildWebpackBrowser(
}
seen.add(file.file);

if (file.name === 'main') {
if (file.name === 'vendor' || (!mainChunkId && file.name === 'main')) {
// tslint:disable-next-line: no-non-null-assertion
mainChunkId = file.id!.toString();
}
Expand Down
124 changes: 115 additions & 9 deletions packages/angular_devkit/build_angular/src/dev-server/index.ts
Expand Up @@ -12,11 +12,7 @@ import {
WebpackLoggingCallback,
runWebpackDevServer,
} from '@angular-devkit/build-webpack';
import {
json,
logging,
tags,
} from '@angular-devkit/core';
import { json, logging, tags } from '@angular-devkit/core';
import { NodeJsSyncHost } from '@angular-devkit/core/node';
import { existsSync, readFileSync } from 'fs';
import * as path from 'path';
Expand Down Expand Up @@ -58,6 +54,46 @@ const devServerBuildOverriddenKeys: (keyof DevServerBuilderOptions)[] = [
'deployUrl',
];

async function createI18nPlugins(
locale: string,
translation: unknown | undefined,
missingTranslation?: 'error' | 'warning' | 'ignore',
) {
const plugins = [];
// tslint:disable-next-line: no-implicit-dependencies
const localizeDiag = await import('@angular/localize/src/tools/src/diagnostics');

const diagnostics = new localizeDiag.Diagnostics();

if (translation) {
const es2015 = await import(
// tslint:disable-next-line: trailing-comma no-implicit-dependencies
'@angular/localize/src/tools/src/translate/source_files/es2015_translate_plugin'
);
plugins.push(
// tslint:disable-next-line: no-any
es2015.makeEs2015TranslatePlugin(diagnostics, translation as any, { missingTranslation }),
);

const es5 = await import(
// tslint:disable-next-line: trailing-comma no-implicit-dependencies
'@angular/localize/src/tools/src/translate/source_files/es5_translate_plugin'
);
plugins.push(
// tslint:disable-next-line: no-any
es5.makeEs5TranslatePlugin(diagnostics, translation as any, { missingTranslation }),
);
}

const inlineLocale = await import(
// tslint:disable-next-line: trailing-comma no-implicit-dependencies
'@angular/localize/src/tools/src/translate/source_files/locale_plugin'
);
plugins.push(inlineLocale.makeLocalePlugin(locale));

return { diagnostics, plugins };
}

export type DevServerBuilderOutput = DevServerBuildOutput & {
baseUrl: string;
};
Expand All @@ -69,6 +105,7 @@ export type DevServerBuilderOutput = DevServerBuildOutput & {
* @param transforms A map of transforms that can be used to hook into some logic (such as
* transforming webpack configuration before passing it to webpack).
*/
// tslint:disable-next-line: no-big-function
export function serveWebpackBrowser(
options: DevServerBuilderOptions,
context: BuilderContext,
Expand Down Expand Up @@ -119,14 +156,83 @@ export function serveWebpackBrowser(
browserName,
);

const webpackConfigResult = await buildBrowserWebpackConfigFromContext(
const { config, projectRoot, i18n } = await buildBrowserWebpackConfigFromContext(
browserOptions,
context,
host,
true,
);
let webpackConfig = config;

const tsConfig = readTsconfig(browserOptions.tsConfig, context.workspaceRoot);
if (i18n.shouldInline && tsConfig.options.enableIvy !== false) {
if (i18n.inlineLocales.size > 1) {
throw new Error(
'The development server only supports localizing a single locale per build',
);
}

const locale = [...i18n.inlineLocales][0];
const translation = i18n.locales[locale] && i18n.locales[locale].translation;

const { plugins, diagnostics } = await createI18nPlugins(
locale,
translation,
browserOptions.i18nMissingTranslation,
);

// No differential loading for dev-server, hence there is just one config
let webpackConfig = webpackConfigResult.config;
// Get the insertion point for the i18n babel loader rule
// This is currently dependent on the rule order/construction in common.ts
// A future refactor of the webpack configuration definition will improve this situation
// tslint:disable-next-line: no-non-null-assertion
const rules = webpackConfig.module!.rules;
const index = rules.findIndex(r => r.enforce === 'pre');
if (index === -1) {
throw new Error('Invalid internal webpack configuration');
}

const i18nRule: webpack.Rule = {
test: /\.(?:m?js|ts)$/,
enforce: 'post',
use: [
{
loader: 'babel-loader',
options: {
babelrc: false,
compact: false,
cacheCompression: false,
plugins,
},
},
],
};

rules.splice(index, 0, i18nRule);

// Add a plugin to inject the i18n diagnostics
// tslint:disable-next-line: no-non-null-assertion
webpackConfig.plugins!.push({
// tslint:disable-next-line:no-any
apply: (compiler: webpack.Compiler) => {
compiler.hooks.thisCompilation.tap('build-angular', compilation => {
compilation.hooks.finishModules.tap('build-angular', () => {
if (!diagnostics) {
return;
}
for (const diagnostic of diagnostics.messages) {
if (diagnostic.type === 'error') {
compilation.errors.push(diagnostic.message);
} else {
compilation.warnings.push(diagnostic.message);
}
}

diagnostics.messages.length = 0;
});
});
},
});
}

const port = await checkPort(options.port || 0, options.host || 'localhost', 4200);
const webpackDevServerConfig = (webpackConfig.devServer = buildServerConfig(
Expand All @@ -145,7 +251,7 @@ export function serveWebpackBrowser(
webpackConfig,
webpackDevServerConfig,
port,
projectRoot: webpackConfigResult.projectRoot,
projectRoot,
};
}

Expand Down
Expand Up @@ -40,7 +40,7 @@ function emittedFilesToInlineOptions(
es5,
outputPath,
missingTranslation,
setLocale: emittedFile.name === 'main',
setLocale: emittedFile.name === 'main' || emittedFile.name === 'vendor',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible for the locale to need to be set in lazy chunks?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the point of view of Angular, we only need to set it in main.js so that we can pick it up in Angular services and pipes. And I guess, since it is global, that once it is set it will automatically be available to all lazy loaded chunks.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then what's the situation where it needs to be set in vendor?

};
originalFiles.push(originalPath);

Expand Down
15 changes: 11 additions & 4 deletions tests/legacy-cli/e2e/tests/i18n/ivy-localize-dl.ts
@@ -1,11 +1,10 @@
import { appendToFile, expectFileToMatch } from '../../utils/fs';
import { ng } from '../../utils/process';
import { execAndWaitForOutputToMatch, killAllProcesses, ng } from '../../utils/process';
import { updateJsonFile } from '../../utils/project';
import { expectToFail } from '../../utils/utils';
import { baseDir, externalServer, langTranslations, setupI18nConfig } from './legacy';


export default async function () {
export default async function() {
// Setup i18n tests and config.
await setupI18nConfig();

Expand Down Expand Up @@ -37,9 +36,12 @@ export default async function () {
// await expectFileToMatch(`${outputPath}/main-es5.js`, '.ng.common.locales');
// await expectFileToMatch(`${outputPath}/main-es2015.js`, '.ng.common.locales');

// Execute Application E2E tests with dev server
await ng('e2e', `--configuration=${lang}`, '--port=0');
clydin marked this conversation as resolved.
Show resolved Hide resolved

// Execute Application E2E tests for a production build without dev server
const server = externalServer(outputPath);
try {
// Execute without a devserver.
await ng('e2e', `--configuration=${lang}`, '--devServerTarget=');
} finally {
server.close();
Expand All @@ -57,4 +59,9 @@ export default async function () {
await expectFileToMatch(`${baseDir}/fr/main-es5.js`, /Other content/);
await expectFileToMatch(`${baseDir}/fr/main-es2015.js`, /Other content/);
await expectToFail(() => ng('build'));
try {
await execAndWaitForOutputToMatch('ng', ['serve', '--port=0'], /No translation found for/);
} finally {
killAllProcesses();
}
}
5 changes: 4 additions & 1 deletion tests/legacy-cli/e2e/tests/i18n/ivy-localize-es2015.ts
Expand Up @@ -26,9 +26,12 @@ export default async function() {
await expectFileNotToExist(`${outputPath}/main-es5.js`);
await expectFileToMatch(`${outputPath}/main.js`, 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);
try {
// Execute without a devserver.
await ng('e2e', `--configuration=${lang}`, '--devServerTarget=');
} finally {
server.close();
Expand Down
5 changes: 4 additions & 1 deletion tests/legacy-cli/e2e/tests/i18n/ivy-localize-es5.ts
Expand Up @@ -25,9 +25,12 @@ export default async function() {
await expectFileNotToExist(`${outputPath}/main-es2015.js`);
await expectFileToMatch(`${outputPath}/main.js`, 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);
try {
// Execute without a devserver.
await ng('e2e', `--configuration=${lang}`, '--devServerTarget=');
} finally {
server.close();
Expand Down
3 changes: 3 additions & 0 deletions tests/legacy-cli/e2e/tests/i18n/ivy-localize-server.ts
Expand Up @@ -10,6 +10,9 @@ import { langTranslations, setupI18nConfig } from './legacy';
const snapshots = require('../../ng-snapshot/package.json');

export default async function () {
// TODO: Re-enable pending further Ivy/Universal/i18n work
return;

// Setup i18n tests and config.
await setupI18nConfig();

Expand Down
45 changes: 25 additions & 20 deletions tests/legacy-cli/e2e/tests/i18n/legacy.ts
Expand Up @@ -25,7 +25,7 @@ export const langTranslations = [
translation: {
helloPartial: 'Bonjour',
hello: 'Bonjour i18n!',
plural: 'Mis à jour Il y a 3 minutes',
plural: 'Mis à jour il y a 3 minutes',
date: 'janvier',
},
translationReplacements: [
Expand All @@ -34,7 +34,7 @@ export const langTranslations = [
['Updated', 'Mis à jour'],
['just now', 'juste maintenant'],
['one minute ago', 'il y a une minute'],
['other {', 'other {Il y a'],
[/other {/g, 'other {il y a '],
['minutes ago', 'minutes'],
],
},
Expand All @@ -52,7 +52,7 @@ export const langTranslations = [
['Updated', 'Aktualisiert'],
['just now', 'gerade jetzt'],
['one minute ago', 'vor einer Minute'],
['other {', 'other {vor'],
[/other {/g, 'other {vor '],
['minutes ago', 'Minuten'],
],
},
Expand Down Expand Up @@ -91,12 +91,12 @@ export async function setupI18nConfig(useLocalize = true) {

// Add e2e specs for each lang.
for (const { lang, translation } of langTranslations) {
await writeFile(`./src/app.${lang}.e2e-spec.ts`, `
await writeFile(`./e2e/src/app.${lang}.e2e-spec.ts`, `
import { browser, logging, element, by } from 'protractor';

describe('workspace-project App', () => {
const getParagraph = (name: string) => element(by.css('app-root p#' + name)).getText();
beforeEach(() => browser.get(browser.baseUrl););
beforeEach(() => browser.get(browser.baseUrl));
afterEach(async () => {
// Assert that there are no errors emitted from the browser
const logs = await browser.manage().logs().get(logging.Type.BROWSER);
Expand All @@ -112,7 +112,7 @@ export async function setupI18nConfig(useLocalize = true) {
expect(getParagraph('locale')).toEqual('${lang}'));

it('should display localized date', () =>
expect(getParagraph('date')).toEqual('${translation.plural}'));
expect(getParagraph('date')).toEqual('${translation.date}'));

it('should display pluralized message', () =>
expect(getParagraph('plural')).toEqual('${translation.plural}'));
Expand Down Expand Up @@ -190,13 +190,13 @@ export async function setupI18nConfig(useLocalize = true) {
if (lang != sourceLocale) {
await copyFile('src/locale/messages.xlf', `src/locale/messages.${lang}.xlf`);
for (const replacements of translationReplacements) {
await replaceInFile(`src/locale/messages.${lang}.xlf`, replacements[0], replacements[1]);
await replaceInFile(`src/locale/messages.${lang}.xlf`, replacements[0], replacements[1] as string);
}
}
}

if (useLocalize) {
// Install the localize package.
// Install the localize package if using ivy
if (!getGlobalVariable('argv')['ve']) {
let localizeVersion = '@angular/localize@' + readNgVersion();
if (getGlobalVariable('argv')['ng-snapshots']) {
localizeVersion = require('../../ng-snapshot/package.json').dependencies['@angular/localize'];
Expand All @@ -209,23 +209,28 @@ export default async function () {
// Setup i18n tests and config.
await setupI18nConfig(false);

// Legacy option usage with the en-US locale needs $localize when using ivy
// Legacy usage did not need to process en-US and typically no i18nLocale options were present
// This will currently be the overwhelmingly common scenario for users updating existing projects
if (!getGlobalVariable('argv')['ve']) {
await appendToFile('src/polyfills.ts', `import '@angular/localize/init';`);
}

// Build each locale and verify the output.
for (const { lang, translation, outputPath } of langTranslations) {
await ng('build', `--configuration=${lang}`);
await expectFileToMatch(`${outputPath}/main-es5.js`, translation.helloPartial);
await expectFileToMatch(`${outputPath}/main-es2015.js`, translation.helloPartial);

// E2E to verify the output runs and is correct.
if (getGlobalVariable('argv')['ve']) {
await ng('e2e', `--configuration=${lang}`);
} else {
const server = externalServer(outputPath);
try {
// Execute without a devserver.
await ng('e2e', `--configuration=${lang}`, '--devServerTarget=');
} finally {
server.close();
}
// Execute Application E2E tests with dev server
await ng('e2e', `--configuration=${lang}`, '--port=0');
clydin marked this conversation as resolved.
Show resolved Hide resolved

// Execute Application E2E tests for a production build without dev server
const server = externalServer(outputPath);
try {
await ng('e2e', `--configuration=${lang}`, '--devServerTarget=');
} finally {
server.close();
}
}

Expand Down
8 changes: 0 additions & 8 deletions tests/legacy-cli/e2e_runner.ts
Expand Up @@ -97,14 +97,6 @@ if (argv.ve) {
// Remove Ivy specific tests
allTests = allTests
.filter(name => !name.includes('tests/i18n/ivy-localize-'));
} else {
// These tests are disabled on the Ivy CI jobs because:
// - Ivy doesn't support the functionality yet
// - The test itself is not applicable to Ivy
// As we transition into using Ivy as the default this list should be reassessed.
allTests = allTests
// Ivy doesn't support i18n externally at the moment.
.filter(name => !name.endsWith('tests/build/aot/aot-i18n.ts'));
}

const shardId = 'shard' in argv ? argv['shard'] : null;
Expand Down