Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ngProgress injected is not the service but the factory #120

Open
toutpt opened this issue May 24, 2016 · 6 comments
Open

ngProgress injected is not the service but the factory #120

toutpt opened this issue May 24, 2016 · 6 comments
Labels

Comments

@toutpt
Copy link

toutpt commented May 24, 2016

capture d ecran 2016-05-24 a 11 38 13

That is weird I have installed it that way:

package.json

"ngprogress": "VictorBjelkholm/ngProgress#v1.1.3",

then include resources:

            "node_modules/ngprogress/build/ngProgress.min.js",
            "node_modules/ngprogress/ngProgress.css",

app.js

angular.module('swif.framework', [...
    'ngProgress',
]);

the controller is on the screen shot

Was working on 1.0.7, do not work on 1.1.3 (also tried on 1.1.1)

Any idea ? I think this has to be with the way you handle injection.

You should name the function and then add $inject property on it.

function ngProgressFactory(x,y,z) {}
ngProgressFactory.$inject('x','y','z');
Because you use a factory and return ['x', f(x) {}] I get that array instead of the service.

@cetra3
Copy link
Collaborator

cetra3 commented May 25, 2016

I'd need to see your controller to find out what is going wrong

You should inject the ngProgressFactory:

var MainCtrl = function($scope, $timeout, ngProgressFactory) {}

Then in your controller's function, create an instance of ngProgress:

$scope.progressbar = ngProgressFactory.createInstance();

@toutpt
Copy link
Author

toutpt commented May 25, 2016

I think it's more a bug than a question because it works on 1.0.

I have injected 'ngProgress' into the controller:

function msgService($uibModal, notificationService, ngProgress, errorMsgService, gettextCatalog) {
    'ngInject';
    ...
    cf capture for the code

If I look to the source: https://github.com/VictorBjelkholm/ngProgress/blob/master/src/provider.js#L3

.service('ngProgress', function () {

So It should be possible to use 'ngProgress' but it's not.
Because it returns something that is not a service:

return ['$document', '$window', '$compile', '$rootScope', '$timeout', function($document, $window, $c

And as I said It works on the 1.0 but not on the 1.1
and I have found the commit that broke my code:

f888e28

So it means 'ngProgress' is not a service anymore ?

Now I need to change from 'ngProgress' to 'ngProgressFactory', create an instance and put it into the scope. You may add a note for this migration path from 1.0 to 1.1
or fix it to be usable as a service.

@cetra3
Copy link
Collaborator

cetra3 commented May 25, 2016

Yeah, it may be worthwhile as a note in the readme for upgrading. I have a feeling it was changed to allow multiple instances of ngProgress at any given time.

@toutpt
Copy link
Author

toutpt commented May 25, 2016

A question, do that work on 1.0 ?

--- a/src/framework/messages.js
+++ b/src/framework/messages.js
@@ -106,9 +106,10 @@
     * @name swif.framework.msgService
     * @description this service provide an API to display feedback to the user.
     **/
-    function msgService($uibModal, notificationService, ngProgress, errorMsgService, gettextCatalog) {
+    function msgService($uibModal, notificationService, ngProgressFactory, errorMsgService, gettextCatalog) {
         'ngInject';
         var messages = [];
+        var ngProgress = ngProgressFactory.createInstance();

@toutpt
Copy link
Author

toutpt commented May 25, 2016

I understand, adding features is always great but breaking changes are bad
(at least 1.0 -> 1.1 should be minor changes).

To handle this I'm using https://github.com/semantic-release/semantic-release which pump major release depending on your commit message and that is great !

@cetra3
Copy link
Collaborator

cetra3 commented May 25, 2016

Yep, agree. This was before my time. Glad to have solved the mystery though.

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

No branches or pull requests

2 participants