lephyrus / ngx-translate-messageformat-compiler

Advanced pluralization (and more) for ngx-translate, using standard ICU syntax which is compiled with the help of messageformat.js.
MIT License
93 stars 29 forks source link

Cannot read property 'toString' of undefined after upgrade to 4.0.0 #26

Closed pascallaprade-beslogic closed 6 years ago

pascallaprade-beslogic commented 6 years ago

This is a redux of issue #25. (But I think the cause is different)

We upgraded to Angular 6 last week, but kept some of our dependencies out of sync until today, among them this library.

Here is the stack trace I get:

ERROR TypeError: Cannot read property 'toString' of undefined
    at _stringify (runtime.js:97)
    at _stringify (runtime.js:103)
    at Runtime.push../node_modules/messageformat/lib/runtime.js.Runtime.toString (runtime.js:118)
    at MessageFormat.push../node_modules/messageformat/lib/messageformat.js.MessageFormat.compile (messageformat.js:343)
    at TranslateMessageFormatCompiler.push../node_modules/ngx-translate-messageformat-compiler/esm5/ngx-translate-messageformat-compiler.js.TranslateMessageFormatCompiler.compileTranslations (ngx-translate-messageformat-compiler.js:25)
    at TranslateService.push../node_modules/@ngx-translate/core/esm5/ngx-translate-core.js.TranslateService.setTranslation (ngx-translate-core.js:373)
    at UserManagementTranslation.push../ClientApp/app/components/user-management/user-management.translation.ts.UserManagementTranslation.init (user-management.translation.ts:14)
    at new UserProfileService (user-profile.service.ts:14)
    at _createClass (core.js:9266)
    at _createProviderInstance$1 (core.js:9234)

The difference I see between this issue and the one mentioned earlier is that this happens not inside the templates, but at the component creation.

We use the translateService.setTranslation method from ngx-translate, wrap it in an init method and call that one up in the component's constructor.

In the stacktrace, this is what we see: first, Angular creates the class, which calls the init method of our class containing our translations, which calls setTranslation, then it goes into TranslateMessageFormatCompiler.compileTranslations and eventually fails in messageformat.

I may be wrong, but it looks to me like if messageformat was trying to actually obtain the result of the string, but since we are not actually using it, our translation parameter is undefined, and toString does not exist on it.

I commented out our translation that used the ICU syntax, and everything worked correctly. I enabled it back again, and everything stopped working.

Here is the code that causes the issue for us:

this.translateService.setTranslation(
  'en-CA',
  USER_MANAGEMENT: {
    X_USERS_IN_GROUP: '{userCount} {userCount, plural, one{user} other{users}} in this group',
  },
  true,
)

Again, if I do not use the ICU syntax, I get no error.

I looked in the changelogs and didn't see a change to the syntax, but I guess it is also possible that our string is now malformed after the update.

So the only clue I have is that setTranslation would start a pipeline resulting in attempting to actually translate the string, even though we don't provide any parameter, which results in an undefined value upon which toString is called.

I can provide more details if you need!

For now, we will be reverting to the previous version: even though it is advertised as not compatible with Angular 6 and ngx-translate 10, and produce warnings at installation, everything works fine.

lephyrus commented 6 years ago

The solution in #25 was to upgrade all packages. Are you sure you provide all your libraries' peer dependencies at the required versions?

Since, like you say, nothing relevant has changed for the new version of this library, have you checked if the implementation of setTranslation has undergone changes for v10 of ngx-translate/core?

If that doesn't get you anywhere, please provide a minimal reproduction of the problem on StackBlitz, thanks.

lephyrus commented 6 years ago

Also, I've just published v4.1.0, which exports TranslateMessageFormatDebugCompiler, which you may use instead of the regular compiler. The console logs should help you figure out if the an attempt to use the interpolation function (and actually get a translated string) is made.

pascallaprade-beslogic commented 6 years ago

Hmm, I started a minimal project locally to see if our dependencies interferred, but it seems there was a problem when you deployed 4.1.0:

When I simply import the package, the module is not found. There is no index in the package's root. If I try to import /src instead, I then get this error:

Module build failed: Error: C:\Users\[...]\Documents\tests\ngx-translate-bug\node_modules\ngx-translate-messageformat-compiler\src\message-format-config.ts is missing from the TypeScript compilation. Please make sure it is in your tsconfig via the 'files' or 'include' property.
The missing file seems to be part of a third party library. TS files in published libraries are often a sign of a badly packaged library. Please open an issue in the library repository to alert its author and ask them to package the library using the Angular Package Format (https://goo.gl/jB3GVv).

Here is what's in the npm-installed package folder:

image

I will try again with 4.0.0, but I thought I would let you know!

lephyrus commented 6 years ago

@pascallaprade-beslogic Damn, I messed up the publishing command. New release (v4.1.1) is now "online" - sorry about that and thanks for the heads-up!

pascallaprade-beslogic commented 6 years ago

Alright, so here is what I have:

Sadly, updating our dependencies did nothing, and this is reproducible in a minimal setup.

I have looked at the code in @ngx-translate/core, and it seems like the file was moved and indentation changed between 9.0.0 and 10.0.0, but that's all. This is not surprising, since having @ngx-translate/core@10.0.0 and ngx-translate-messageformat-compiler@3.0.0 worked fine. If the fault was in @ngx-translate/core, I would have expected for the code not to work as soon as we updated it from 9.0.0.

I am thinking that the problem may be in messageformat, then. Maybe compile should not be called during the compileTranslations phase? I'm not super familiar with messageformat, so it's hard to say, but, yeah, maybe that's a possibility.

I have created a minimal project on StackBlitz: https://stackblitz.com/edit/angular-lzumok

I used the 4.1.1 there, so I can use the DebugCompiler, and the stack trace now looks like so:

[TranslateMessageFormatCompiler] COMPILE (en-CA) {}
[TranslateMessageFormatCompiler] COMPILE (en-CA) {userManagement: {…}}
ERROR Error: o is undefined
Error: o is undefined

I believe the o is undefined is the same thing I get, but focuses on the object instead of the property that is attempted to be read. It may be related to how the console works in StackBlitz vs Firefox's.

In app.translation.ts, you can toggle between a string using ICU syntax and one that does not, and you will see that the one with ICU crashes the app, while the one without is fine and works.

Thanks for your time and help, by the way!

lephyrus commented 6 years ago

@pascallaprade-beslogic Your error is not related to this library or ngx-translate overall, it's purely a messageformat issue. If you downgrade messageformat to v1, your example works. If you switch the language code to en, your example works even with the current version.

I think this is a minimal reproduction: https://stackblitz.com/edit/typescript-cqpntn

pascallaprade-beslogic commented 6 years ago

Thank you such much!

Carniatto commented 5 years ago

hello @lephyrus I'm using ngx-translate-messageformat-compiler: 4.3.0 and I have the following problems:

Can we either correct the package.json or fix the compatibility with MessageFormat v2?

lephyrus commented 5 years ago

@Carniatto If your issue is indeed the same as described: the issue is well understood and you'll find an explanation and a solution in the referenced PR above as well as in the README.