Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
30 changes: 29 additions & 1 deletion __tests__/buildTranslationFiles.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,9 @@ type TranslationCategory =
| 'multi-input'
| 'scope-mapping'
| 'comments'
| 'remove-extra-keys';
| 'remove-extra-keys'
| 'self-closing'
| 'control-flow';

interface assertTranslationParams extends Pick<Config, 'fileFormat'> {
type: TranslationCategory;
Expand Down Expand Up @@ -238,6 +240,32 @@ describe.each(formats)('buildTranslationFiles in %s', (fileFormat) => {
});
});

describe('Self-closing', () => {
const type: TranslationCategory = 'self-closing';
const config = gConfig(type);

beforeEach(() => removeI18nFolder(type));

it('should work with self-closing component', () => {
let expected = generateKeys({ end: 4 });
createTranslations(config);
assertTranslation({ type, expected, fileFormat });
});
});

describe('Control flow', () => {
const type: TranslationCategory = 'control-flow';
const config = gConfig(type);

beforeEach(() => removeI18nFolder(type));

it('should work with control flow', () => {
let expected = generateKeys({ end: 26 });
createTranslations(config);
assertTranslation({ type, expected, fileFormat });
});
});

describe('read', () => {
const type: TranslationCategory = 'read';
const config = gConfig(type);
Expand Down
79 changes: 79 additions & 0 deletions __tests__/control-flow/1.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
<span>{{ '1' | transloco }}</span>
<span transloco="2"></span>
<ng-container *transloco="let t">
<span>{{ t('3') }}</span>
</ng-container>
<ng-template transloco let-t>
<span>{{ t('4') }}</span>
</ng-template>


@if (a > b) {
<span>{{ '5' | transloco }}</span>
<span transloco="6">
</span>
<ng-container *transloco="let t">
<span>{{ t('7') }}</span>
</ng-container>
<ng-template transloco let-t>
<span>{{ t('8') }}</span>
</ng-template>
} @else if (b > a) {
<span>{{ '9' | transloco }}</span>
} @else {
<span [innerHtml]="'10' | transloco"></span>
}

@for (item of items; track item.id) {
{{ '11' | transloco }}
} @empty {
<span>{{ '12' | transloco }}</span>
}

@switch (condition) {
@case (caseA) {
<span>{{ '13' | transloco }}</span>
}
@default {
<span>{{ '14' | transloco }}</span>
}
}

@defer {
<span>{{ '15' | transloco }}</span>
} @error {
<span>{{ '16' | transloco }}</span>
} @placeholder {
<span>{{ '17' | transloco }}</span>
} @loading {
<span>{{ '18' | transloco }}</span>
}

<ng-container *transloco="let t">
@if (a > b) {
<span>{{ t('19') }}</span>
} @else if (b > a) {
<span [innerHtml]="t('20')"></span>
} @else {
@for (item of items; track item.id) {
<span>{{ t('21') }}</span>
} @empty {
@switch (condition) {
@case (caseA) {
<span>{{ t('22') }}</span>
}
@default {
@defer {
<span>{{ '23' | transloco }}</span>
} @error {
<span transloco="24"></span>
} @placeholder {
<span>{{ t('25') }}</span>
} @loading {
<span>{{ t('26') }}</span>
}
}
}
}
}
</ng-container>
8 changes: 8 additions & 0 deletions __tests__/self-closing/1.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<self-closing [value]="'1' | transloco" />
<self-closing transloco="2" />
<ng-container *transloco="let t">
<self-closing [value]="t('3')" />
</ng-container>
<ng-template transloco let-t>
<self-closing [value]="t('4')" />
</ng-template>
9 changes: 9 additions & 0 deletions src/keys-builder/template/directive.extractor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ import { resolveAliasAndKey } from '../utils/resolvers.utils';

import { TemplateExtractorConfig } from './types';
import {
getChildrendNodesIfBlock,
isBlockWithChildren,
isBoundAttribute,
isBoundText,
isConditionalExpression,
isElement,
isInterpolation,
Expand All @@ -30,6 +33,12 @@ export function directiveExtractor(config: TemplateExtractorConfig) {

function traverse(nodes: TmplAstNode[], config: ExtractorConfig) {
for (const node of nodes) {
const childrendNodes = getChildrendNodesIfBlock(node);
if (childrendNodes.length) {
traverse(childrendNodes, config);
continue;
}

if (!isSupportedNode(node, [isTemplate, isElement])) {
continue;
}
Expand Down
7 changes: 7 additions & 0 deletions src/keys-builder/template/pipe.extractor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
isPropertyRead,
isTemplate,
parseTemplate,
getChildrendNodesIfBlock,
} from './utils';

export function pipeExtractor(config: TemplateExtractorConfig) {
Expand All @@ -27,6 +28,12 @@ export function pipeExtractor(config: TemplateExtractorConfig) {

function traverse(nodes: TmplAstNode[], config: ExtractorConfig) {
for (const node of nodes) {
const childrendNodes = getChildrendNodesIfBlock(node);
if (childrendNodes.length) {
traverse(childrendNodes, config);
continue;
}

let astTrees: AST[] = [];

if (isElement(node) || isTemplate(node)) {
Expand Down
7 changes: 7 additions & 0 deletions src/keys-builder/template/structural-directive.extractor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
isSupportedNode,
isTemplate,
parseTemplate,
getChildrendNodesIfBlock,
} from './utils';

interface ContainerMetaData {
Expand All @@ -49,6 +50,12 @@ export function traverse(
config: TemplateExtractorConfig,
) {
for (const node of nodes) {
const childrendNodes = getChildrendNodesIfBlock(node);
if (childrendNodes.length) {
traverse(childrendNodes, containers, config);
continue;
}

let methodUsages: ContainerMetaData[] = [];

if (isBoundText(node)) {
Expand Down
80 changes: 80 additions & 0 deletions src/keys-builder/template/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,17 @@ import {
TmplAstElement,
TmplAstTemplate,
TmplAstTextAttribute,
TmplAstNode,
TmplAstDeferredBlock,
TmplAstDeferredBlockError,
TmplAstDeferredBlockLoading,
TmplAstDeferredBlockPlaceholder,
TmplAstForLoopBlock,
TmplAstForLoopBlockEmpty,
TmplAstIfBlockBranch,
TmplAstSwitchBlockCase,
TmplAstIfBlock,
TmplAstSwitchBlock,
} from '@angular/compiler';

import { readFile } from '../../utils/file.utils';
Expand Down Expand Up @@ -98,3 +109,72 @@ export function isSupportedNode<Predicates extends any[]>(
): node is GuardedType<Predicates[number]> {
return predicates.some((predicate) => predicate(node));
}

export function isBlockWithChildren(
node: unknown,
): node is { children: TmplAstNode[] } {
return (
node instanceof TmplAstDeferredBlockError ||
node instanceof TmplAstDeferredBlockLoading ||
node instanceof TmplAstDeferredBlockPlaceholder ||
node instanceof TmplAstForLoopBlockEmpty ||
node instanceof TmplAstIfBlockBranch ||
node instanceof TmplAstSwitchBlockCase
);
}

export function isTmplAstForLoopBlock(
node: unknown,
): node is TmplAstForLoopBlock {
return node instanceof TmplAstForLoopBlock;
}

export function isTmplAstDeferredBlock(
node: unknown,
): node is TmplAstDeferredBlock {
return node instanceof TmplAstDeferredBlock;
}

export function isTmplAstIfBlock(node: unknown): node is TmplAstIfBlock {
return node instanceof TmplAstIfBlock;
}

export function isTmplAstSwitchBlock(
node: unknown,
): node is TmplAstSwitchBlock {
return node instanceof TmplAstSwitchBlock;
}

function isNotNull<T>(input: T | null): input is T {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can be removed, see the next comments

Copy link
Contributor Author

@kekel87 kekel87 Mar 11, 2024

Choose a reason for hiding this comment

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

It's a great tool for solving typing problems in strict mode. See #180 (comment) and #180 (comment)

Copy link
Collaborator

@shaharkazaz shaharkazaz Mar 11, 2024

Choose a reason for hiding this comment

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

K I see.
Let's a positive form of this like isTmplAstNode instead of isNotNull

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

return input !== null;
}

/**
* Returns children nodes of the given node if it's a block node.
*/
export function getChildrendNodesIfBlock(node: TmplAstNode): TmplAstNode[] {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please separate this into isBlockNode and resolveBlockChildren
Once separated we can make node.children the default return value of resolveBlockChildren and have something like this:

    if (isBlockNode(node)) {
      traverse(resolveBlockChildNodes(node), containers, config);
      continue;
    }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This means we'll have to run the if tests twice. Not very optimizing, is it?

What's more, if I return node.children without doing an if, I get a typing error.

export function isBlockNode(node: TmplAstNode): boolean {
  // test here for the first time
  return isTmplAstIfBlock(node) ||
    isTmplAstForLoopBlock(node) ||
    isTmplAstDeferredBlock(node) ||
    isTmplAstSwitchBlock(node) ||
    isBlockWithChildren(node);
}

export function resolveBlockChildNodes(node: TmplAstNode): TmplAstNode[] {
  // test again
  if (isTmplAstIfBlock(node)) {
    return node.branches;
  }

  if (isTmplAstForLoopBlock(node)) {
    return node.empty ? [...node.children, node.empty] : node.children;
  }

  if (isTmplAstDeferredBlock(node)) {
    // Typing errors here
    return [
      ...node.children,
      ...[node.loading, node.error, node.placeholder].filter(Boolean),
    ];
  }

  if (isTmplAstSwitchBlock(node)) {
    return node.cases;
  }

   // Typing errors here
  return node.children;
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. We don't have any performance considerations here and this isn't some very heavy calculation that I would agree we need to optimize, I'd take readability.
  2. What's the type error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

if (isTmplAstIfBlock(node)) {
return node.branches;
}

if (isTmplAstForLoopBlock(node)) {
return [...node.children, ...[node.empty].filter(isNotNull)];
}

if (isTmplAstDeferredBlock(node)) {
return [
...node.children,
...[node.loading, node.error, node.placeholder].filter(isNotNull),
];
}

if (isTmplAstSwitchBlock(node)) {
return node.cases;
}

if (isBlockWithChildren(node)) {
return node.children;
}

return [];
}