search

gsklee / ngStorage

localStorage and sessionStorage done right for AngularJS.
MIT License
2.33k stars 461 forks source link

feat(webpack/browserify): final fix 😂 #159

Closed robinboehm closed 8 years ago

robinboehm commented 9 years ago

Hey @egilkh,

First: Thanks for this! :)

2nd: to enable the easy import like:

angular.module('App', [
    require('angular-ui-router'),
    require('ngstorage')
]);

You need to export the module name as string, not the module :)

egilkh commented 9 years ago

Hey! :)

Won't this break requirejs (and/or others) ? As UMD doesn't seem to follow this convention. If I remember correctly this https://github.com/umdjs/umd/blob/master/commonjsStrict.js was the basis for support of UMD/CommonJS.

From an Angular view point it doesn't matter which every as long as it gets to module definition. But I do believe requirejs/others expect the module and not the name of the module to be returned. Which does make a whole lot more sense then returning a string back imho.

dpogue commented 8 years ago

The standard for other Angular modules is to export the name of the module as a string. See https://github.com/angular/angular.js/pull/10732#issuecomment-75225380

ctaepper commented 8 years ago

+1

egilkh commented 8 years ago

Makes sense to do this. Guess this will be 0.4.0 and break backwards for some.

But since (hopefully) no one relied on the returned module in their Angular code it should be fine I think. Added to my todo :)

tarlepp commented 8 years ago

+1

egilkh commented 8 years ago

It is merged. Will do a release in a few days I think.

moisesrodriguez commented 7 years ago

@egilkh Will you do a release anytime soon?

tarlepp commented 7 years ago

What is the hold up with this one ?