atticoos / angular-translate-once

:currency_exchange: Extension of angular-translate for one time bindings
53 stars 11 forks source link

Use of bind polyfill is required for testing of anything that uses angular-translate-once #4

Closed vladgurovich closed 8 years ago

vladgurovich commented 8 years ago

Looks like unit tests of any code that uses angular-translate-once fail with

TypeError: 'undefined' is not a function (evaluating 'TranslateOnceAttributeDirective.bind(undefined, attribute)')

at line https://github.com/ajwhite/angular-translate-once/blob/master/src/translate-once.js#L16 unless your Function.prototype.bind polyfill is added, which is not ideal for a third party library.

Do you have another strategy of avoiding above error while testing? why use a function that requires a polyfill?

atticoos commented 8 years ago

This started as an extension in a company product which uses bind. It satisfies a large range of supported browsers, and usually only fails in PhantomJS 1.9 or lower since anything prior to PhantomJS2 uses an earlier QT framework without the bind support.

We felt comfortable using .bind since it met our minimum browser requirements. As this becomes more broadly adopted, I'm happy to revisit.

vladgurovich commented 8 years ago

Its odd, since karma-phantomjs-launcher has not been been updated to support phantomjs2. What do you use to run your tests?

kasperlewau commented 8 years ago

You could override the PhantomJS version used by karma-phantomjs-launcher by setting an environment variable. Which very well could be what the author has done.

Taken from the karma docs:

# Changing the path to the Chrome binary
$ export CHROME_BIN=/usr/local/bin/my-chrome-build

# Changing the path to the Chrome Canary binary
$ export CHROME_CANARY_BIN=/usr/local/bin/my-chrome-build

# Changing the path to the PhantomJs binary
$ export PHANTOMJS_BIN=$HOME/local/bin/phantomjs

If neither that or the usage of a .bind polyfill floats the boat, I think your best bet would be to fork karma-phantomjs-launcher and make the necessary changes to use PhantomJS 2.x.

Just my two cents.

vladgurovich commented 8 years ago

We actually ran into a lot of trouble running PhantomJS2 on our CI instances(even though they call 2.0 stable, i see a lot of issues and regressions on https://github.com/ariya/phantomjs/labels/2.0). I think we will have to resort to using the polyfill or fork this project and use a common library like lodash instead.

atticoos commented 8 years ago

bind is really only used to make this part easier: https://github.com/ajwhite/angular-translate-once/blob/develop/src/translate-once.js#L74-L76

Might as well just clean this bit up to construct the directive without it https://github.com/ajwhite/angular-translate-once/blob/develop/src/translate-once.js#L15-L16

atticoos commented 8 years ago

v1.0.3 has been released with the changes :+1: