Circular dependency when using inside $http interceptor

[suhrab]

This article http://blog.altoros.com/speed-up-i18n-in-angularjs.html
describes how to speed up angular applications that heavily use translate libraries like this one. Article gives example for angular-translate. I wanted it work with angular-gettext.

In example they use $translate service from angular-translate as dependency to factory that creates request interceptor. It works.

I replaced $translate with gettextCatalog, but now I get an error from angular about circular dependency: https://docs.angularjs.org/error/$injector/cdep?p0=$http%20%3C-%20gettextCatalog%20%3C-%20translateHttpInterceptor%20%3C-%20$http%20%3C-%20$templateFactory%20%3C-%20$view%20%3C-%20$state

The problem:
angular-gettext factory 'gettextCatalog' depends on $http (loadRemote method). if I remove this dependency and 'loadRemote' method, everything works fine.

Actually I think that angular-gettext should not depend on $http, it is very easy to write my own loading from remote method using $http and 'gettextCatalog.setStrings'.

angular-translate does not depend on $http

[rubenv]

Easy solution around circular dependencies: inject $injector and use var $http = $injector.get("$http");.

[gabegorelick]

Making gettextCatalog a provider would probably fix this. See #117.

[gabegorelick]

@suhrab Please take a look at #140 and make sure it fixes this for you. Thanks.

[suhrab]

Sorry for late answer. #140 does not fix circular dep.

Edit
It does fix it. But to use it I must write my interceptor inside module config function:

app.config([
        '$httpProvider',
        'gettextCatalogProvider',
        appConfig
    ]);

    app.run([
        '$couchPotato',
        '$rootScope',
        '$state',
        '$stateParams',
        'gettextCatalog',
        appRun
    ]);

    return app;

    function appConfig($httpProvider, gettextCatalogProvider){
        $httpProvider.interceptors.push(function(){
            return {
                'request': function(config) {
                    return config;
                },

                'response': function(response) {
                    //_.templateSettings = {
                    //    'interpolate': /\[\[([\s\S]+?)]]/g
                    //};

                    if (typeof response.data === 'string') {
                        response.data = _.template(response.data, {t: gettextCatalogProvider});
                    }
                    return response;
                }
            }
        });
    }

I want to use it inside a factory, like this:

/**
 * Created by Anton on 05.12.2014.
 */
define([
    'modules/common/module',
    'lodash'
], function (commonModule, _) {
    commonModule.factory('translateHttpInterceptor', [
        'gettextCatalogProvider',
        translateHttpInterceptor
    ]);

    function translateHttpInterceptor(gettextCatalogProvider){
        console.log(gettextCatalogProvider)
        return {
            'request': function(config) {
                return config;
            },

            'response': function(response) {
                //_.templateSettings = {
                //    'interpolate': /\[\[([\s\S]+?)]]/g
                //};

                if (typeof response.data === 'string') {
                    response.data = _.template(response.data, {t: gettextCatalogProvider});
                }
                return response;
            }
        };
    }
});

Any suggestions?

[gabegorelick]

@suhrab Sorry about that. Try it now and see if it works.

[frederikprijck]

Any updates on this ? Still open, and the issue still exists.

This single method realy makes people struggle with using this in things like intercepters, config functions etc.

What is the added value of the loadRemote method to be part of the same service ? All it does is make an ajax call, which is easily done outside the service. One method to populate a service, should not be part of the same service, simple seperation of concerns tell us one object should not be responsible for both being the catalog and loading external resources.

Theoreticly, Moving the method from the gettextCatalog, to a new service 'gettextCatalogRemote' (e.g.) which has a dependency to $http and gettextCatalog should do the trick, making the method's implementation :

[rubenv]

I agree, it's a bit unfortunate this got added (though it does keep things simple for many people).
Can't be removed without breaking API either.

[frederikprijck]

I understand, however apart from totally liking gettext I think keeping this inside the service will pull people away from it (or atleast those who are aware of the issue) while I rly think this thing is realy good.

All it does is make it simple for people who are less aware of what's supposed to be happening (mixing concerns is oké aslong as it doesn't gives you problems). I don't want to be dependend on $http when all I want to do is ask for the current language, that's totally unacceptable, neither should I be dependend on $http to control a translations catalog, but this latter is not the biggest issue. There is a big difference between "keeping things simple for some people" and "limiting others"

My problem is this:

• I have a LanguageService, dependend on gettextCatalog
• I have a LanguageInterceptor to make sure the language get's added to the http headers, which is dependend on LanguageService. (All this does is calling languageService.get() which calls getCurrentLanguage of the gettextCatalog)

What happens is the following:

• Angular registers gettextCatalog and resolves the $http dependency.
• While resolving $http, angular resolves all registered interceptors
• As my interceptor requires languageService (to get the current language), the languageService is resolved by angular.
• As the languageService is dependend on gettextCatalog, angular resolved gettextCatalog and we are back at the start (circular references).

Injecting $injector and using the Service Locator pattern doesn't work (or I'm missing something).

Currently, I do not see a proper workaround besides storing the current language on a different place (one to simply set/get the language from memory).
As all my language modifications are using the LanguageService, both the memoryLanguage and gettextCatalog services are controller by this languageService. So the downside isn't very big, but it's pitty that we have the need to create such a workaround because gettext doesn't support the most basic feature of a translation library: getting the current language without being dependend on irrelevant things.

PS: breaking ur API is not good. But they have invented major versioning for a reason. Keeping this inside your service is far more worse than eventualy going to a new major version. So this shouldn't be a valid reason to keep mistakes in your service. (I call it a mistake, as you also called it unfortunate it got in there).

EDIT: I'm pretty oké with migrating away from gettextCatalog to get the current language. I do think it may even be cleaner.

So my new flow is:

• Set the language using LanguageService which has NO dependency on gettextCatalog
• Use pub/sub to notify my application of the languageChange
• Create a run function to listen for language changed event and update the language of the gettextCatalog

I did not remove my comment tho, as I still think the comments I made are valid.

[Zirand]

I need to setup 'Accept-Language' header. however I must do it via httpProvider.interceptors. And i get lang frоm appConfig. And You are not belive but I am setup gettextCatalog.setCurrentLanguage(lang) in appConfig [sarcasm]!!! And Miracle!!!
[$injector:cdep] Circular dependency found: gettextCatalog < - appConfig < - $http < - gettextCatalog

:-(

[MigFerreira]

Having the same problem with the $httpInterceptor... would really love this to be solved as i cant seem to get around it. Maybe best option is changing to another translation solution.

[frederikprijck]

@Zirand and @MigFerreira have you tried my workaround, mentioned above ? I got it working ...

[MigFerreira]

@Zirand, @frederikprijck and @rubenv I actually removed $http direct dependency from gettextCatalog by changing loadRemote to:

loadRemote: function (url) {
            return $injector.get('$http')({
                method: 'GET',
                url: url,
                cache: catalog.cache
            }).then(function (response) {
                var data = response.data;
                for (var lang in data) {
                    catalog.setStrings(lang, data[lang]);
                }
                return response;
            });
}

Did this 10 min ago, and seems to be working but need to do some more tests.

[frederikprijck]

Well IIRC I tried this aswell and it didn't work. But maybe you're having better luck. Please to keep us informed on your tests !

[MigFerreira]

@frederikprijck @rubenv @Zirand, Ok tested all my use cases and my solution works, don't know if its a 100% solution for all possible use cases.

My problematic use cases:

• languageService loading json remotely when user changes language in preferences.
• httpInterceptor translating api errors based on the response codes matching a service of errorConstants.
  The errorConstants service was using gettextCatalog and was generating the circular dependency.

Now works fine.

I really hate changing external lib source, when i have more time i will try your solution @frederikprijck.

[frederikprijck]

I'm nowhere saying that my solution is better then urs. I highly doubt it. Your solution requires less effort, however it does require modification of source code which I don't do unless it's being accepted as PR (or unless there realy is no other way around it)...