Skip to content

Commit

Permalink
fix: Find uses of translate pipe in pipe arguments (bartholomej#2)
Browse files Browse the repository at this point in the history
* refactor(pipe-parser) simplify handling of pipes in getTranslatablesFromAst

remove expressionIsOrHasBindingPipe - previously this also handled cases like `'World' | translate | upper`,
but it's not actually needed, getTranslatablesFromAst can deal with it by itself

move checks for translate and non-translate pipes together under a common BindingPipe check.

* fix(pipe-parser): find usages of translate pipe in arguments of other pipes
  • Loading branch information
P4 authored Mar 2, 2023
1 parent 23786a6 commit 8e7a3fe
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 21 deletions.
34 changes: 13 additions & 21 deletions src/parsers/pipe.parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,19 @@ export class PipeParser implements ParserInterface {
}

protected getTranslatablesFromAst(ast: AST): BindingPipe[] {
// the entire expression is the translate pipe, e.g.:
// - 'foo' | translate
// - (condition ? 'foo' : 'bar') | translate
if (this.expressionIsOrHasBindingPipe(ast)) {
return [ast];
if (ast instanceof BindingPipe) {
// the entire expression is the translate pipe, e.g.:
// - 'foo' | translate
// - (condition ? 'foo' : 'bar') | translate
if (ast.name === TRANSLATE_PIPE_NAME) {
// also visit the pipe arguments - interpolateParams object
return [ast, ...this.getTranslatablesFromAsts(ast.args)];
}

// not the translate pipe - ignore the pipe, visit the expression and arguments, e.g.:
// - { foo: 'Hello' | translate } | json
// - value | date: ('mediumDate' | translate)
return this.getTranslatablesFromAsts([ast.exp, ...ast.args]);
}

// angular double curly bracket interpolation, e.g.:
Expand All @@ -124,12 +132,6 @@ export class PipeParser implements ParserInterface {
}
}

// a pipe on the outer expression, but not the translate pipe - ignore the pipe, visit the expression, e.g.:
// - { foo: 'Hello' | translate } | json
if (ast instanceof BindingPipe) {
return this.getTranslatablesFromAst(ast.exp);
}

// object - ignore the keys, visit all values, e.g.:
// - { key1: 'value1' | translate, key2: 'value2' | translate }
if (ast instanceof LiteralMap) {
Expand Down Expand Up @@ -157,16 +159,6 @@ export class PipeParser implements ParserInterface {
return [].concat(...array);
}

protected expressionIsOrHasBindingPipe(exp: any): exp is BindingPipe {
if (exp.name && exp.name === TRANSLATE_PIPE_NAME) {
return true;
}
if (exp.exp && exp.exp instanceof BindingPipe) {
return this.expressionIsOrHasBindingPipe(exp.exp);
}
return false;
}

protected parseTemplate(template: string, path: string): TmplAstNode[] {
return parseTemplate(template, path).nodes;
}
Expand Down
12 changes: 12 additions & 0 deletions tests/parsers/pipe.parser.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,18 @@ describe('PipeParser', () => {
expect(keys).to.deep.equal([]);
});

it('should extract translate pipe used as pipe argument', () => {
const contents = `{{ value | valueToTranslationKey: ('argument' | translate) }}`;
const keys = parser.extract(contents, templateFilename).keys();
expect(keys).to.deep.equal(['argument']);
});

it('should extract nested uses of translate pipe', () => {
const contents = `{{ 'Hello' | translate: {world: ('World' | translate)} }}`;
const keys = parser.extract(contents, templateFilename).keys();
expect(keys).to.deep.equal(['Hello', 'World']);
});

it('should extract strings from piped arguments inside a function calls on templates', () => {
const contents = `{{ callMe('Hello' | translate, 'World' | translate ) }}`;
const keys = parser.extract(contents, templateFilename).keys();
Expand Down

0 comments on commit 8e7a3fe

Please sign in to comment.