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.

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

Jump to bottom

fix(core): handle empty translations correctly #36499

Closed
wants to merge 2 commits into from
Closed

fix(core): handle empty translations correctly #36499

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2cf3fa9
fix(core): handle empty translations correctly
AndrewKushnir Apr 8, 2020
512f5a8
fixup! fix(core): handle empty translations correctly
AndrewKushnir Apr 9, 2020
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
159 changes: 88 additions & 71 deletions packages/core/src/render3/i18n.ts
View file
Open in desktop
Expand Up @@ -243,7 +243,7 @@ function removeInnerTemplateTranslation(message: string): string {
* external template and removes all sub-templates.
*/
export function getTranslationForTemplate(message: string, subTemplateIndex?: number) {
if (typeof subTemplateIndex !== 'number') {
if (isRootTemplateMessage(subTemplateIndex)) {
// We want the root template message, ignore all sub-templates
return removeInnerTemplateTranslation(message);
} else {
Expand Down Expand Up @@ -376,6 +376,10 @@ export function ɵɵi18nStart(index: number, message: string, subTemplateIndex?:
// This is reset to 0 when `i18nStartFirstPass` is called.
let i18nVarsCount: number;

function allocNodeIndex(startIndex: number): number {
return startIndex + i18nVarsCount++;
}

/**
* See `i18nStart` above.
*/
Expand Down Expand Up @@ -409,81 +413,90 @@ function i18nStartFirstPass(
const updateOpCodes: I18nUpdateOpCodes = [];
const icuExpressions: TIcu[] = [];

const templateTranslation = getTranslationForTemplate(message, subTemplateIndex);
const msgParts = replaceNgsp(templateTranslation).split(PH_REGEXP);
for (let i = 0; i < msgParts.length; i++) {
let value = msgParts[i];
if (i & 1) {
// Odd indexes are placeholders (elements and sub-templates)
if (value.charAt(0) === '/') {
// It is a closing tag
if (value.charAt(1) === TagType.ELEMENT) {
const phIndex = parseInt(value.substr(2), 10);
parentIndex = parentIndexStack[--parentIndexPointer];
createOpCodes.push(phIndex << I18nMutateOpCode.SHIFT_REF | I18nMutateOpCode.ElementEnd);
}
} else {
const phIndex = parseInt(value.substr(1), 10);
const isElement = value.charAt(0) === TagType.ELEMENT;
// The value represents a placeholder that we move to the designated index.
// Note: positive indicies indicate that a TNode with a given index should also be marked as
// parent while executing `Select` instruction.
createOpCodes.push(
(isElement ? phIndex : ~phIndex) << I18nMutateOpCode.SHIFT_REF |
I18nMutateOpCode.Select,
parentIndex << I18nMutateOpCode.SHIFT_PARENT | I18nMutateOpCode.AppendChild);

if (isElement) {
parentIndexStack[++parentIndexPointer] = parentIndex = phIndex;
}
}
} else {
// Even indexes are text (including bindings & ICU expressions)
const parts = extractParts(value);
for (let j = 0; j < parts.length; j++) {
if (j & 1) {
// Odd indexes are ICU expressions
const icuExpression = parts[j] as IcuExpression;

// Verify that ICU expression has the right shape. Translations might contain invalid
// constructions (while original messages were correct), so ICU parsing at runtime may not
// succeed (thus `icuExpression` remains a string).
if (typeof icuExpression !== 'object') {
throw new Error(`Unable to parse ICU expression in "${templateTranslation}" message.`);
if (message === '' && isRootTemplateMessage(subTemplateIndex)) {
// If top level translation is an empty string, do not invoke additional processing
// and just create op codes for empty text node instead.
createOpCodes.push(
message, allocNodeIndex(startIndex),
parentIndex << I18nMutateOpCode.SHIFT_PARENT | I18nMutateOpCode.AppendChild);
} else {
const templateTranslation = getTranslationForTemplate(message, subTemplateIndex);
const msgParts = replaceNgsp(templateTranslation).split(PH_REGEXP);
for (let i = 0; i < msgParts.length; i++) {
let value = msgParts[i];
if (i & 1) {
// Odd indexes are placeholders (elements and sub-templates)
if (value.charAt(0) === '/') {
// It is a closing tag
if (value.charAt(1) === TagType.ELEMENT) {
const phIndex = parseInt(value.substr(2), 10);
parentIndex = parentIndexStack[--parentIndexPointer];
createOpCodes.push(phIndex << I18nMutateOpCode.SHIFT_REF | I18nMutateOpCode.ElementEnd);
}

// Create the comment node that will anchor the ICU expression
const icuNodeIndex = startIndex + i18nVarsCount++;
} else {
const phIndex = parseInt(value.substr(1), 10);
const isElement = value.charAt(0) === TagType.ELEMENT;
// The value represents a placeholder that we move to the designated index.
// Note: positive indicies indicate that a TNode with a given index should also be marked
// as parent while executing `Select` instruction.
createOpCodes.push(
COMMENT_MARKER, ngDevMode ? `ICU ${icuNodeIndex}` : '', icuNodeIndex,
(isElement ? phIndex : ~phIndex) << I18nMutateOpCode.SHIFT_REF |
I18nMutateOpCode.Select,
parentIndex << I18nMutateOpCode.SHIFT_PARENT | I18nMutateOpCode.AppendChild);

// Update codes for the ICU expression
const mask = getBindingMask(icuExpression);
icuStart(icuExpressions, icuExpression, icuNodeIndex, icuNodeIndex);
// Since this is recursive, the last TIcu that was pushed is the one we want
const tIcuIndex = icuExpressions.length - 1;
updateOpCodes.push(
toMaskBit(icuExpression.mainBinding), // mask of the main binding
3, // skip 3 opCodes if not changed
-1 - icuExpression.mainBinding,
icuNodeIndex << I18nUpdateOpCode.SHIFT_REF | I18nUpdateOpCode.IcuSwitch, tIcuIndex,
mask, // mask of all the bindings of this ICU expression
2, // skip 2 opCodes if not changed
icuNodeIndex << I18nUpdateOpCode.SHIFT_REF | I18nUpdateOpCode.IcuUpdate, tIcuIndex);
} else if (parts[j] !== '') {
const text = parts[j] as string;
// Even indexes are text (including bindings)
const hasBinding = text.match(BINDING_REGEXP);
// Create text nodes
const textNodeIndex = startIndex + i18nVarsCount++;
createOpCodes.push(
// If there is a binding, the value will be set during update
hasBinding ? '' : text, textNodeIndex,
parentIndex << I18nMutateOpCode.SHIFT_PARENT | I18nMutateOpCode.AppendChild);
if (isElement) {
parentIndexStack[++parentIndexPointer] = parentIndex = phIndex;
}
}
} else {
// Even indexes are text (including bindings & ICU expressions)
const parts = extractParts(value);
for (let j = 0; j < parts.length; j++) {
if (j & 1) {
// Odd indexes are ICU expressions
const icuExpression = parts[j] as IcuExpression;

// Verify that ICU expression has the right shape. Translations might contain invalid
// constructions (while original messages were correct), so ICU parsing at runtime may
// not succeed (thus `icuExpression` remains a string).
if (typeof icuExpression !== 'object') {
throw new Error(
`Unable to parse ICU expression in "${templateTranslation}" message.`);
}

if (hasBinding) {
addAllToArray(generateBindingUpdateOpCodes(text, textNodeIndex), updateOpCodes);
// Create the comment node that will anchor the ICU expression
const icuNodeIndex = allocNodeIndex(startIndex);
createOpCodes.push(
COMMENT_MARKER, ngDevMode ? `ICU ${icuNodeIndex}` : '', icuNodeIndex,
parentIndex << I18nMutateOpCode.SHIFT_PARENT | I18nMutateOpCode.AppendChild);

// Update codes for the ICU expression
const mask = getBindingMask(icuExpression);
icuStart(icuExpressions, icuExpression, icuNodeIndex, icuNodeIndex);
// Since this is recursive, the last TIcu that was pushed is the one we want
const tIcuIndex = icuExpressions.length - 1;
updateOpCodes.push(
toMaskBit(icuExpression.mainBinding), // mask of the main binding
3, // skip 3 opCodes if not changed
-1 - icuExpression.mainBinding,
icuNodeIndex << I18nUpdateOpCode.SHIFT_REF | I18nUpdateOpCode.IcuSwitch, tIcuIndex,
mask, // mask of all the bindings of this ICU expression
2, // skip 2 opCodes if not changed
icuNodeIndex << I18nUpdateOpCode.SHIFT_REF | I18nUpdateOpCode.IcuUpdate, tIcuIndex);
} else if (parts[j] !== '') {
const text = parts[j] as string;
// Even indexes are text (including bindings)
const hasBinding = text.match(BINDING_REGEXP);
// Create text nodes
const textNodeIndex = allocNodeIndex(startIndex);
createOpCodes.push(
// If there is a binding, the value will be set during update
hasBinding ? '' : text, textNodeIndex,
parentIndex << I18nMutateOpCode.SHIFT_PARENT | I18nMutateOpCode.AppendChild);

if (hasBinding) {
addAllToArray(generateBindingUpdateOpCodes(text, textNodeIndex), updateOpCodes);
}
}
}
}
Expand Down Expand Up @@ -558,6 +571,10 @@ function appendI18nNode(
return tNode;
}

function isRootTemplateMessage(subTemplateIndex: number|undefined): subTemplateIndex is undefined {
return subTemplateIndex === undefined;
}

/**
* Handles message string post-processing for internationalization.
*
Expand Down
35 changes: 35 additions & 0 deletions packages/core/test/acceptance/i18n_spec.ts
View file
Open in desktop
Expand Up @@ -1611,6 +1611,41 @@ onlyInIvy('Ivy i18n logic').describe('runtime i18n', () => {
});
});

describe('empty translations', () => {
it('should replace existing text content with empty translation', () => {
loadTranslations({[computeMsgId('Some Text')]: ''});
const fixture = initWithTemplate(AppComp, '<div i18n>Some Text</div>');
expect(fixture.nativeElement.textContent).toBe('');
});

it('should replace existing DOM elements with empty translation', () => {
loadTranslations({
[computeMsgId(
' Start {$START_TAG_DIV}DIV{$CLOSE_TAG_DIV}' +
'{$START_TAG_SPAN}SPAN{$CLOSE_TAG_SPAN} End ')]: '',
});
const fixture = initWithTemplate(AppComp, `
<div i18n>
Start
<div>DIV</div>
<span>SPAN</span>
End
</div>
`);
expect(fixture.nativeElement.textContent).toBe('');
});

it('should replace existing ICU content with empty translation', () => {
loadTranslations({
[computeMsgId('{VAR_PLURAL, plural, =0 {zero} other {more than zero}}')]: '',
});
const fixture = initWithTemplate(AppComp, `
<div i18n>{count, plural, =0 {zero} other {more than zero}}</div>
`);
expect(fixture.nativeElement.textContent).toBe('');
});
});

it('should work with directives and host bindings', () => {
let directiveInstances: ClsDir[] = [];

Expand Down