alexcrack / angular-ui-notification

Angular.js service providing simple notifications using Bootstrap 3 styles with css transitions for animating
MIT License
536 stars 169 forks source link

Use explicit dependency injection for the provider in the source file #44

Closed vtertre closed 8 years ago

vtertre commented 8 years ago

The fact that explicit dependency injection is not used for the provider in the source file is causing problems when using browserify & ng-annotate, which is blocking.

It requires the provider to be created with:

this.$get = ['$timeout', '$http', '$compile', '$templateCache', '$rootScope', '$injector', '$sce', '$q', '$window',
        function($timeout, $http, $compile, $templateCache, $rootScope, $injector, $sce, $q, $window) {
              ...
        }
]

It is done in your minified file but the source file is used with browserify.

vtertre commented 8 years ago

After looking at the other packages I use, the recommended strategy is to release two different files for your package. A minified version (as it is already the case) but also an unminified version. This is what is done with all the other packages I can see.

I managed to make it work by adding another gulp task:

gulp.src(['src/*.js'])
      .pipe(jshint())
      .pipe(jshint.reporter('default'))
      .pipe(jshint.reporter('fail'))
      .pipe(ngAnnotate())
      .pipe(addsrc('build/*.js'))
      .pipe(order([
          'src/*.js',
          'build/angular-ui-notification.templates.js'
      ]))
      .pipe(concat('angular-ui-notification.js'))
      .pipe(header(banner, { pkg : pkg }))
      .pipe(gulp.dest('dist'));
});

I just copied the service task and removed the minification and demo destination instructions but maybe you can come up with something better as I don't know my way around gulp that much.

Sorry for being a bit needy but this is preventing me from releasing to production so if you could have a look at it in the near future it would be great.

vtertre commented 8 years ago

Actually, this can be done just by adding two lines to the service task before uglification.

gulp.task('service', function() {
    gulp.src(['src/*.js'])
        .pipe(jshint())
        .pipe(jshint.reporter('default'))
        .pipe(jshint.reporter('fail'))
        .pipe(ngAnnotate())
        .pipe(addsrc('build/*.js'))
        .pipe(order([
            'src/*.js',
            'build/angular-ui-notification.templates.js'
        ]))
        .pipe(concat('angular-ui-notification.js'))

        .pipe(header(banner, { pkg : pkg }))
        .pipe(gulp.dest('dist'))

        .pipe(uglify())
        .pipe(rename({
            suffix: '.min'
        }))
        .pipe(header(banner, { pkg : pkg }))
        .pipe(gulp.dest('dist'))
        .pipe(gulp.dest('demo'));
});
alexcrack commented 8 years ago

Merged