Using t binding behaviour inside view attached with <compose /> produces i18next::translator: missingKey warnings
valichek opened this issue · 16 comments
Here is gist.run that uses latest aurelia and i18n
https://gist.run/?id=f7ead986372293a1e558e0932c6e1ec6
I think this issue is closely related to #79
Well I'm not sure thats a real issue. If I remove the compose part from your code and just leave the first section you'll anyway get those warnings because what i18next tries is to load information from the defaultNS which is translation. As it won't find the dynamically created one ${'val1'+getLabel(prop) & t}
inside the translation file it will issue the warning. Still though the translation happens because it finds it in the alternate namespace.
So what is it that you think is the exact bug here?
@zewa666 sorry, example was not correct, I have updated it (same link). I use Chrome and get following log:
i18next::translator: missingKey de translation DE-tr1 DE-tr1
i18next::translator: missingKey de translation DE DE
i18next::translator: missingKey de translation DE-tr1-test DE-tr1-test
i18next::translator: missingKey de translation DE-tr1-test DE-tr1-test
Note that it tries to translate already translated values (for usual ${'val1' & t}
too), but it happens only when using two parts of example if the same key is used in composed view too. I have more complicated structure in my app though. I just created the basic example to show the issue.
Here is different very simplified version of example: https://gist.run/?id=464debee45d962d88793708615517697
Here's an even more fun gist: is this something to do with caching?
https://gist.run/?id=4460ebe5d8e1fd274767f72005ea5cba
If you change from en to de and back again you can see that the translations get all screwy
It looks like whatever stores the translations is being updated incorrectly
Edit: actually in debugging, it looks like if you have multiple binding behaviours the translate is called multiple times (multiplied by the number of binding behaviours you have)
Here's an example of it all working without using the i18n behaviour:
https://gist.run/?id=4460ebe5d8e1fd274767f72005ea5cba
I've just used the standard signal behaviour and the t
value converter, so it must be something to do with the t
behaviour.
Not sure why though as the t
behaviour just rewrites the binding with the t
value converter and signal
behaviour right?
Having a lot of fun debugging :)
So it boils down to the fact that there are side effects to rewriting the binding.
https://gist.run/?id=4460ebe5d8e1fd274767f72005ea5cba
(I bump the expression chain back 1 step in tbind.js)
The above "fixes" it - so from what I can tell, the first rewrite of the binding expression messes up the second rewrite. The second rewrite actually chains the previous evaluation and then the second rewrite adds another step to this chain - and so on.
You end up with all identical bindings being chained multiple times.
Edit: here's another example of it working correctly for two of the 4 bindings because I've made them unique by applying additional throttle and debounce behaviours to them:
https://gist.run/?id=40f27d8a8e709b22f2f0d0fa0b51141d
This probably happens due to caching at some level (optimisations maybe in the AST?) for identical bindings.
Unfortunately, this is a very possible use case, you can work around simpler scenarios by ensuring the bindings have some sort of uniquifier:
${ val1 & t }
and
${ (val1) & t }
etc.
I suppose the best approach would be to check if the binding had already been rewritten somehow and don't rewrite it again - maybe push an i18n prop of some sort onto the binding and check for it when re-writing...
Here's an example of that (seems to work on the bind, not tried testing the unbind)
https://gist.run/?id=8bce7221102742e91aebcfe6c287a648
p.s. @jdanyow gist.run is awesome!
@jdanyow Perhaps you can provide some feedback on this?
Here's what I think we can do to fix this: https://gist.run/?id=be99269ff5295eb3872ff4cea419e3c6
Take a look at the last revision to that gist, it has two changes to the TBindingBehavior:
- Adds a boolean prop to the sourceExpression to track whether it's been re-written. This prevents double/triple/quadruple/etc rewrites.
- On unbind, don't undo the expression rewrite
The alternative to this would be to use the new ExpressionCloner so we can do the re-writing with a fresh expression. This would enable restoring the binding to it's original state on unbind but I don't think this is really necessary in this case.
Thoughts?
@jdanyow I like that approach. @charlespockert and @valichek what do you guys think about this?
Hi, having the same issue. Any update on this ?
Done. @EisenbergEffect can you include this one as well into the new release?
Sorry, what do I need to do exactly? 😆
Just a note for next tuesdays release day so we can check wheter this issue got fixed with the recent release
@zewa666 No release on Tuesday. Official 1.0 release is scheduled for Wednesday.
@valichek, @swiftanthony and @charlespockert the new release is published, please check out whether it fixes your issue
closing due to inactivity