angular-ui / ui-leaflet

AngularJS directive to embed an interact with maps managed by Leaflet library
http://angular-ui.github.io/ui-leaflet
Other
314 stars 134 forks source link

update to 0.9.0 fails for debug dependency, bower when bundled via browserify #133

Open nmccready opened 9 years ago

nmccready commented 9 years ago

From @nschloe on October 16, 2015 14:34

At the last update of leaflet (to 0.8), we've had problems since the new simple-logger dependency wasn't available in bower. We're going through the same thing for 0.9 because apparently simple-logger now depends on debug which isn't available for bower.

Copied from original issue: tombatossals/angular-leaflet-directive#997

nmccready commented 9 years ago

Which js file are you pointing to for the logger? You should be using angular-simple-logger.js or angular-simple-logger.light.js as they don't require debug as it is wrapped or not included. Otherwise use npm and use browserify/webpack.

nmccready commented 9 years ago

From @nschloe on October 16, 2015 15:25

We're just using the leaflet directive which depends on logger which now depends on debug. We're managing all of that through bower/browserify, but like I said, it's become somewhat painful to maintain the dependencies.

nmccready commented 9 years ago

So if your using browserify, don't use bower for this library. Use the npm version. The visionmedia debug, part is what makes this an issue as its browser version will break browserify. Just as the bower version of angular-simple-logger.js is made with browserify already and will break commonjs if used. So use the commonJS version of index.js. The dependencies are propagated correctly on npm.

Visionmedia's debug has issues in bower where only using the browser version or browserifying it for bower (angular-simple-logger) will work. This is becuase they decided to not include any of their dependencies which are on npm. https://github.com/visionmedia/debug/blob/2.2.0/bower.json

From their point of view using the https://github.com/visionmedia/debug/blob/2.2.0/browser.js is sufficient. Which I am essentially doing the same by browserifying angular-simple-logger.js for bower.

I don't really know any other way around this sorry. But npm is your better choice or use the light version with no debug.

nmccready commented 9 years ago

To discuss this more please file an issue on angular-simple-logger.

nmccready commented 9 years ago

The other problem is I don't have perms yet to push angular-leaflet-directive to npm. If I did then you could get this project from there and avoid this problem.

I am adding some comments about this to the Readme.md on angular-simple-logger.

nmccready commented 9 years ago

989 @tombatossals :boom:

nmccready commented 9 years ago

https://github.com/nmccready/angular-simple-logger/blob/master/README.md

nmccready commented 9 years ago

One possible improvement to avoid your personal h3ll, is to make the *light.js version the bower default. But be aware you will not have debug capabilities.

nmccready commented 9 years ago

@nschloe does any of this make sense?

nmccready commented 9 years ago

From @nschloe on October 16, 2015 19:8

@nmccready , thanks for sharing your insight.

So if your using browserify, don't use bower for this library. Use the npm version.

The existence of a bower.json for the leaflet directive had lured us into thinking that this use case was actually supported. Since we're not using npm for frontend shipping dependencies and manually maintaining all dependencies of leaflet is no acceptable for us, we'll just remove the leaflet dependency altogether from our app.

As a developer, I don't understand why a directive has to ship it's own logging library, especially since, eventually, this is the reason why we can't use the directive. Having a logging tool on the dev box for debugging purposes: alright. But baked into the product? The way it got into leaflet without any review also strikes me as somewhat dubious and raises concerns on how much headache continuing to use this directive would cause us in the future.

Well, anyhow, he who codes is right, so I won't suggest to change anything.

nmccready commented 9 years ago

So you can use bower, but if your dead set in browserifying every library under the sun from bower say for angular-leaflet-directive. Then your going to run into the problems which you are. So what I am saying to do is split your bower up. Use some as browserify and some as a seperate vendor.js file.

So again if you must browserify all your dependencies (AND want the full logger version) then I suggest using npm. This project plans to support both bower and npm. npm being a little rusty right now (delayed releases).

Again if your deadset on browserifying and expect it to not have errors then direct your logger dependency to the light version.

Ultimately I think the less headache solution will be pointing bower to use the light version. That way it will not error out on ng-leaflet. Then a user can decide if they want the advanced debugging features and use bower, browserify, npm to do its magic.

nmccready commented 9 years ago

I made a ticket; I'll try to get to it by Monday and make this for 0.9.1 here. https://github.com/nmccready/angular-simple-logger/issues/12

nmccready commented 9 years ago

Re-opening for 0.9.1 so we point to angular-simple-logger#>=0.1.5

nmccready commented 9 years ago

As a developer, I don't understand why a directive has to ship it's own logging library, especially since, eventually, this is the reason why we can't use the directive

THIS IS WHY #829 .. I was doing this same code in several projects. Why copy and paste the code?