Bug(messageformat): Plural translation starting and ending with a variables fails

julpellat commented 1 year ago

Is there an existing issue for this?

[X] I have searched the existing issues

Which Transloco package(s) are the source of the bug?

@ngneat/transloco-messageformat

Is this a regression?

No

Current behavior

I have the following translation using plural in my en.json file :

"MONITORING_CALLOUT": "{count, plural, \none {{count} manager n’a pas réalisé sa déclaration de variables à la date d'échéance pour la période de {period}} \nother {{count} n'ont pas réalisé leur déclaration de variables à la date d'échéance pour la période de {period}}\n}"

The page load but nothing show on the screen. Nor the value, nor the translation key. And I have the following error in the console

ERROR Error: invalid syntax at line 2 col 1:

  one  
  ^
    _token messageformat.js:3477
    next messageformat.js:3419
    next messageformat.js:3490
    n messageformat.js:279
    parseSelect messageformat.js:3860
    parseArgToken messageformat.js:3960
    parseBody messageformat.js:3983
    parse messageformat.js:3828
    parse messageformat.js:4044
    compile messageformat.js:4510
    compile messageformat.js:7216
    compile ngneat-transloco-messageformat.mjs:22
    transpile ngneat-transloco-messageformat.mjs:52
    translate ngneat-transloco.mjs:597
    updateValue ngneat-transloco.mjs:1338
    subscription ngneat-transloco.mjs:1325
    RxJS 43
    transform ngneat-transloco.mjs:1325
    pipeBind2 Angular
    MonitoringPage_ng_container_9_div_1_Template monitoring.page.html:21
    Angular 24
    RxJS 6
    Angular 20
    RxJS 16

After some testing my conclusion are :

when there is a variable at the start and at the end the parsing throws an error Example : "{count, plural, \none {bla bla} \nother {{var1} bla bla {var2}}"
If I had a spaces it works fine Like this "{count, plural, \none {bla bla} \nother {{var1} bla bla {var2} }" or at the start "count, plural, \none {bla bla} \nother { {var1} bla bla {var2}}"
If I use the syntax with a # it also works fine "{count, plural, \none {bla bla} \nother {# bla bla {var2}}"

Expected behavior

The syntax "KEY": "count, plural, \none {bla bla} \nother {{var1} bla bla {var2}}" should not throws any error.

Please provide a link to a minimal reproduction of the bug

https://stackblitz.com/edit/transloco-messageformat-giflos?file=src%2Fassets%2Fi18n%2Fen-US.json,src%2Fapp%2Fapp.component.html,src%2Fapp%2Fapp.module.ts,src%2Findex.html

Transloco Config

No response

Please provide the environment you discovered this bug in

"@ngneat/transloco": "^4.2.1"
"@ngneat/transloco-messageformat": "^4.1.0"
Angular: 15.0.4
Node: 18.12.1
Package Manager: npm 8.19.2
OS: Windows 10

Browser

Firefox: version 109.0 (64 bits)

Additional context

No response

I would like to make a pull request for this bug

No

stackblitz[bot] commented 1 year ago

Start a new pull request in StackBlitz Codeflow.

transloco-messageformat-giflos (Open reproduction)

ben12 commented 1 year ago

There is a conflict between transloco and messageformat patterns.

For {count, plural, one {bla bla} other {{var1} bla bla {var2}}}, transloco match {{var1} bla bla {var2}} and search a value for var1} bla bla {var2 which is not found, so messageformat receive {count, plural, one {bla bla} other var1} bla bla {var2} and returns a syntax error.

A workaround is to use another pattern for transloco by replacing the default configuration of interpolation.

The final solution may be to add forbidden characters for parameters name:

export interface TranslocoConfig {

[...]

  interpolation?: [start: string, end: string, forbiddenChars?: string];
}

By default it may be interpolation: ['{{', '}}', "{}"]. So interpolation regex will become {{([^{}]*?)}} instead of {{(.*?)}}. It is a breaking change because '{' and '}' cannot be used anymore for parameters name, but i think that is an error to use them.

It can also be nice to escape regex reserved characters:

 function resolveMatcher(userConfig?: TranslocoConfig): RegExp {
-  const [start, end] =
+  const [start, end, forbiddenChars] =
     userConfig && userConfig.interpolation
       ? userConfig.interpolation
       : defaultConfig.interpolation!;
+  const matchingParamName = forbiddenChars !== undefined ? `[^${escapeForRegExp(forbiddenChars)}]` : '.';
+  return new RegExp(`${escapeForRegExp(start)}(${matchingParamName}*?)${escapeForRegExp(end)}`, 'g');
-  return new RegExp(`${start}(.*?)${end}`, 'g');
}

+function escapeForRegExp(text: string) {
+  return text.replace(/[-[\]{}()*+?.,\\^$|#\s]/g, "\\$&");
+}

What do you think about this ?

shaharkazaz commented 1 year ago

@julpellat What's wrong with the workarounds you provided? You can try @ben12's suggestion and pass a custom interpolation value.

julpellat commented 1 year ago

Hi, sorry for the late response. I’ve tested your solution and it work like a charm. This is exactly what I need.

To answer your question @shaharkazaz I can’t use the workarounds I provided because we use lokalize and lokalize generate the json file used by transloco. Lokalize generate plural under this syntax "{count, plural, \none {bla bla} \nother {{var1} bla bla {var2}}" It will be way easier for us to fix messageformat instead of changing the lokalize config and refactor all the plural syntax in all of our software (since lokalize handle the translation for all of our software)

julpellat commented 1 year ago

Hi, the PR seems to be ready since late march. Is there still some work needed to merge ? Can I help in any way ?

JMCelesti commented 1 year ago

@julpellat I was looking since yesterday all day long and I found the solution not just for you but for the other looking for the solution the none case: =0 is mandatory

I have on my file =0 {} one{just one} other{more than one}

shaharkazaz commented 1 year ago

@julpellat I need to fully understand the use case and make sure this isn't adding a bunch of code to a very specific use case. (seeing @JMCelesti comment) I didn't have time to dive into it, I'll try to take a look when I get the chance.

jsverse / transloco