Skip to content

Commit

Permalink
BUGFIX: Forgiving I18nRegistry.translate for strings with colons
Browse files Browse the repository at this point in the history
The following node type configuration

```yaml
placeholder: 'ClientEval: node.properties.tagName'
```

currently yields an error which is caught by the error boundary for properties:

> Error: TranslationAddress must adhere to format "{packageKey}:{sourceName}:{transUnitId}". Got "ClientEval: node.properties.tagName" instead.

The newly added strictness in neos#3804 introduces this regression here.

Previously the function `getTranslationAddress` would just attempt to spit a string and expect three items to be returned:

https://github.com/neos/neos-ui/blob/2cecfcf136aafcd9e3f72537f513eb81b01e993b/packages/neos-ui-i18n/src/registry/I18nRegistry.js#L7

The deconstruction would already cause an error in PHP but not in JavaScript:

```
const [packageKey, sourceName, id] = 'ClientEval: node.properties.tagName'.split(':')
```

Returns

```
packageKey: "ClientEval"
sourceName: " node.properties.tagName"
id: undefined
```

The previous forgiveness needs to be reintroduced as its easy to create errors with strings containing colons at any position which is definitely likely for placeholders. Imagine: `placeholder: "The title of the blog:"`.
  • Loading branch information
mhsdesign committed Jan 20, 2025
1 parent e4c2283 commit 6aa1bd2
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 5 deletions.
19 changes: 19 additions & 0 deletions packages/neos-ui-i18n/src/model/TranslationAddress.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,23 @@ describe('TranslationAddress', () => {
.becauseStringDoesNotAdhereToExpectedFormat('foo bar')
);
});

it('can be try created from string', () => {
const translationAddress = TranslationAddress.tryFromString(
'Some.Package:SomeSource:some.transunit.id'
);

expect(translationAddress).not.toBeNull();
expect(translationAddress?.id).toBe('some.transunit.id');
expect(translationAddress?.sourceName).toBe('SomeSource');
expect(translationAddress?.packageKey).toBe('Some.Package');
expect(translationAddress?.fullyQualified).toBe('Some.Package:SomeSource:some.transunit.id');
});

it('try with invalid string returns null', () => {
expect(TranslationAddress.tryFromString('foo bar')).toBeNull();
expect(TranslationAddress.tryFromString('something:')).toBeNull();
// error in placeholder https://github.com/neos/neos-ui/pull/3907
expect(TranslationAddress.tryFromString('ClientEval: node.properties.tagName')).toBeNull();
});
});
15 changes: 12 additions & 3 deletions packages/neos-ui-i18n/src/model/TranslationAddress.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,26 @@ export class TranslationAddress {
}): TranslationAddress =>
new TranslationAddress(props.id, props.sourceName, props.packageKey, `${props.packageKey}:${props.sourceName}:${props.id}`);

public static fromString = (string: string): TranslationAddress => {
public static tryFromString = (string: string): TranslationAddress|null => {
const parts = string.split(TRANSLATION_ADDRESS_SEPARATOR);
if (parts.length !== 3) {
throw TranslationAddressIsInvalid
.becauseStringDoesNotAdhereToExpectedFormat(string);
return null;
}

const [packageKey, sourceName, id] = parts;

return new TranslationAddress(id, sourceName, packageKey, string);
}

public static fromString = (string: string): TranslationAddress => {
const translationAddress = TranslationAddress.tryFromString(string);
if (translationAddress === null) {
throw TranslationAddressIsInvalid
.becauseStringDoesNotAdhereToExpectedFormat(string);
}

return translationAddress;
}
}

export class TranslationAddressIsInvalid extends Error {
Expand Down
13 changes: 13 additions & 0 deletions packages/neos-ui-i18n/src/registry/getTranslationAddress.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,17 @@ describe('getTranslationAddress', () => {
expect(translationAddress.sourceName).toBe('SomeSource');
expect(translationAddress.packageKey).toBe('Some.Package');
});

it('try with invalid string returns null', () => {
// error in placeholder https://github.com/neos/neos-ui/pull/3907
const translationAddress = getTranslationAddress(
'ClientEval: node.properties.tagName',
'Some.Package',
'SomeSource'
);

expect(translationAddress.id).toBe('ClientEval: node.properties.tagName');
expect(translationAddress.sourceName).toBe('SomeSource');
expect(translationAddress.packageKey).toBe('Some.Package');
});
});
8 changes: 6 additions & 2 deletions packages/neos-ui-i18n/src/registry/getTranslationAddress.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
*/
import {TranslationAddress} from '../model';

/**
* @deprecated legacy implementation for I18nRegistry we want to use the TranslationAddress instead.
*/
export function getTranslationAddress(
fullyQualifiedTransUnitId: string
): TranslationAddress;
Expand All @@ -22,8 +25,9 @@ export function getTranslationAddress(
packageKey?: string,
sourceName?: string
) {
if (id && id.indexOf(':') !== -1) {
return TranslationAddress.fromString(id);
const fullyQualifiedTranslationAddress = TranslationAddress.tryFromString(id);
if (fullyQualifiedTranslationAddress !== null) {
return fullyQualifiedTranslationAddress;
}

if (packageKey === undefined) {
Expand Down

0 comments on commit 6aa1bd2

Please sign in to comment.