Skip to content
This repository has been archived by the owner on Jan 29, 2024. It is now read-only.

TypeError: Cannot call method 'trim' of undefined in $translate directive #298

Closed
asm89 opened this issue Jan 20, 2014 · 11 comments
Closed
Labels

Comments

@asm89
Copy link

asm89 commented Jan 20, 2014

With the latest canary version of this module we're seeing getting TypeError: Cannot call method 'trim' of undefined errors. We've been able to track the error down to the directive when both translate and translate-values are used, but only if they're used on elements within an ng-switch (I know right 👍).

The error can be seen here: http://jsfiddle.net/uyjLS/.

After this line it is possible for the scope.translationId to be undefined: here. In master there was a check for that here. This is the commit that changed it: 3b1d1c6.

We've now fixed it locally by checking if (value && scope.translationId), but we're not sure if this is the right way to fix it and we don't know how to write an appropriate test for it. Another thing we notice is that in the "failure" case it seems that value is {}.

Hope you guys know what is happening and we're interested in seeing the actual fix so we can do it ourselves next time! :)

@knalli
Copy link
Member

knalli commented Jan 20, 2014

Oh well.. I had actually reviewed 4939424 but thought that could not be happened. Should be more pessimistic.

@PascalPrecht Fix both?

@0x-r4bbit
Copy link
Member

Hey @asm89 !

First of all, thank you for the great issue! You gave us all needed information to get into this. Of course, we'll take care of that.

@knalli Do you have time to fix it?

@asm89
Copy link
Author

asm89 commented Jan 20, 2014

@knalli I see your commit, but isn't the whole problem that it gets called with undefined?

@knalli
Copy link
Member

knalli commented Jan 20, 2014

yeah, but that one was the one which crashed the stuff internally and should be avoided.

@PascalPrecht I'm not sure whether the stricter condition of 08f087b will made the unwatch() still good enough. Otherwise, this should be fixed.

Actually, the bug is because value is always true (empty object). Still not ideal.

scope.$watch('interpolateParams', function (value) {

@0x-r4bbit
Copy link
Member

@knalli if there are no interpolateParams it actually should be undefined, shouldn't it?

@knalli
Copy link
Member

knalli commented Jan 21, 2014

it's an empty object, actually.

@asm89
Copy link
Author

asm89 commented Jan 24, 2014

@PascalPrecht @knalli Any news here? Any way we can help?

We also noticed the empty object, that still yields true in js, maybe that's the error?

@knalli
Copy link
Member

knalli commented Jan 24, 2014

Let's wait for @PascalPrecht because I'm quite not sure what the intent of the change was.

@knalli
Copy link
Member

knalli commented Jan 24, 2014

Okay, that was some kind of optimization. So, actually,

              scope.$watch('interpolateParams', function (value) {
                if (scope.translationId && value) {
                  iElement.html(translate(scope.translationId,  value, translateInterpolation));
                }
              }, true);

should be

              scope.$watch('interpolateParams', function (value) {
                if (scope.translationId) {
                  iElement.html(translate(scope.translationId,  value, translateInterpolation));
                }
              }, true);

because

  1. !{} === false
  2. Even if it is now empty, the dirty watch check means that is was prior not empty. We have to invoke the translation.

imho

@asm89
Copy link
Author

asm89 commented Feb 6, 2014

ping @PascalPrecht

@0x-r4bbit
Copy link
Member

Reapplied @knalli 's patch in canary.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants