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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(webpack/browserify): final fix 馃槀 #159

Closed
wants to merge 1 commit into from

Conversation

robinboehm
Copy link
Contributor

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 :)

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
Copy link
Contributor

egilkh commented Aug 25, 2015

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
Copy link
Contributor

dpogue commented Mar 8, 2016

The standard for other Angular modules is to export the name of the module as a string. See angular/angular.js#10732 (comment)

@ctaepper
Copy link

+1

@egilkh
Copy link
Contributor

egilkh commented Jun 7, 2016

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
Copy link

tarlepp commented Aug 11, 2016

+1

@egilkh egilkh mentioned this pull request Aug 11, 2016
@egilkh egilkh closed this in #237 Aug 11, 2016
egilkh added a commit that referenced this pull request Aug 11, 2016
@egilkh
Copy link
Contributor

egilkh commented Aug 11, 2016

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

@moisesrodriguez
Copy link

@egilkh Will you do a release anytime soon?

@robinboehm robinboehm deleted the patch-1 branch January 13, 2017 10:17
@tarlepp
Copy link

tarlepp commented Jan 17, 2017

What is the hold up with this one ?

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

Successfully merging this pull request may close these issues.

None yet

6 participants