chore(npm): Make modules Browserify compatible

[bclinkinbeard]

Better version of earlier PR.

index.js for angular:

require('./angular');module.exports=angular;

index.js for other modules:

require('./angular-animate');module.exports=angular.module('ngAnimate');

Still working on adding a dependency to angular in package.json of other modules.

[caitp]

i don't really see the benefit if exporting the module object. seems like it would just encourage people to add things to existing modules :\

[caitp]

make sure to comment these lines clearly to indicate that you're transforming the repo name into a module name, alright?

[bclinkinbeard]

Sure, good idea.

[bclinkinbeard]

No, it allows you to pull them in the way you'd expect in a CommonJS workflow. When I write my own modules, I usually create my app structure like this:

angular.module('app',[require('./someModule').name,require('./otherModule').name]);

Copying @btford as he recommended the format.

[caitp]

you could just export the name of the module instead, I don't think this is that great

[bclinkinbeard]

The module is what the file is actually creating though, so in idiomatic CommonJS a developer would expect the module to be returned when requiring it.

[caitp]

I don't really see it this way.

I think a developer would be more likely to expect the angular object to be exported from the angular module, not the module itself. You can do things with the module, but you should not do anything with the module, it doesn't really serve any purpose to you as a developer. The name of the module is all that really matters there

[bclinkinbeard]

Fair enough, updated.

[caitp]

don't you need to pipe these echos to index.js with >>?

[bclinkinbeard]

Good catch, thanks! Fixed.

[bclinkinbeard]

Thinking about the dependency more, angular would actually be a peerDependency of the other modules. That said, peer dependencies are notoriously error prone, and may even be deprecated at some point in the future.

I think we should call this PR done, and people will just need to realize they can't use things like ngAnimate without separately having angular. That seems pretty reasonable to me.

[caitp]

instead of removing index.js, touching it and then appending to it, you could just do this:

echo "require('./$repo');" > index.js --- that would shave off a few lines of code and be a bit easier to read

[bclinkinbeard]

Very nice, done.

[bclinkinbeard]

So you think we do need to add a peerDependency to the modules?

[caitp]

I'll give this a few other comments in the morning, but I guess ngMock is tricky, because it exports 3 different modules, and the module names don't end with an s like the repo name. Worry about that later though. Good work so far!

[bclinkinbeard]

Not making it in for 1.3.9, eh? :(

Thanks for all your help!

[bclinkinbeard]

OK, peerDependencies added to modules, multiple exports for ngMocks, and a bit of clean up. I've tested it a few times now by running the prepare action, and all the generated code looks correct to me.

I am seeing a problem trying to use the modules though. Given the following code, I get the error Uncaught Error: [$injector:unpr] Unknown provider: ngRouteProvider <- ngRoute.

Any ideas?

varangular=require('angular');varmocks=require('angular-mocks');console.log(angular);angular.module('app',[require('angular-route'),mocks.ngMock,mocks.ngMockE2E,mocks.ngAnimateMock]).run(function(ngRoute,ngMock,ngMockE2E,ngAnimateMock){console.log(ngRoute);console.log(ngMock);console.log(ngMockE2E);console.log(ngAnimateMock);});

[bclinkinbeard]

OK, duh, in my rush to prove this out I was using the wrong injection names. This code works as expected!

varangular=require('angular');varmocks=require('angular-mocks');console.log(angular);angular.module('app',[require('angular-route'),//mocks.ngMock,mocks.ngMockE2E,mocks.ngAnimateMock]).run(function($route,$httpBackend,$animate){console.log($route);console.log($httpBackend);console.log($animate);});

Let me know if you'd like me to share my test app.

[caitp]

Let me know if you'd like me to share my test app.

Please do =)

[bclinkinbeard]

Here you go.

It has angular, angular-animate, angular-mocks, and angular-route in the node_modules folder but you can add any of the modules from the tmp folder after running the prepare action from publish.sh.

npm run bundle will generate the bundle, then start a server and open index.html.

[btford]

I highly recommend against using peerDependencies – npm/npm#5080 (comment)

[bclinkinbeard]

Yea, I mention their problems and uncertain future, but @caitp pointed out that since these would be locked to a specific version we shouldn't have any issues. I'm happy to remove it, but that means devs will need to explicitly install angular itself, along with the other modules they need. (Which seems fine, just pointing out.)

[caitp]

well, no, I wrote a comment originally but removed it because of issues with npm that kind of get in the way of it =)

Basically, you need all the angular modules to be for the same version --- buuuuut, because npm does its own thing, every module can have its own versions of the same dependencies... so it doesn't really solve anything :(

peer deps don't really help either =\ it's just one of those things ._.

[bclinkinbeard]

Using peerDependencies like this would actually help, because if one were to npm i angular-route, you would get both angular and angular-route at the top level of your node_modules directory.

That being said, say the word and I will delete that line. :)

[caitp]

@bclinkinbeard so, I think we're definitely going to want to get some feedback from the community on what gets exported, and the sort of conventions that go along with it.

So, we're sort of trying out a new thing to get feedback from the (huge) community of angular users, could I get you to try and follow this document by sending an intent-to-implement message to the mailing list?

The goal of this would be:

1. inform people that CJS support is planned
2. figure out what CJS support should look like (what should be exported, etc)
3. get approval from owners to ship CJS support

Sound good?

[bclinkinbeard]

Sure, I will do that this evening.

[jgoux]

Awesome work, thanks a lot !

[petebacondarwin]

Thanks to all you guys who have contributed to this effort and discussion, especially @bclinkinbeard, @bendrucker, @kentcdodds, @caitp and @btford!

[snapwich]

Good work guys, I think this is great for the Angular community.

I do have some questions regarding CommonJS support and Angular's global export and it seems like this would be the best place to voice them; @caitp let me know if this should be placed somewhere else.

Are there any plans, or could there be plans, to prevent Angular's global export along with this effort for CommonJS compatibility? Most CommonJS libraries do not usually export to window. Also, as is common in the UMD pattern, modules are only exported to window if the CommonJS exports is not detected. So while the way CommonJS is implemented here will make require'ing angular a possibility (awesome, thanks again guys), it won't provide the normal encapsulation one would expect from using a CommonJS library.

This primarily seems to be a concern in the situation of multiple angular's being present on the page. In my situation at @needle our application is injected into third-party pages which may or may not already contain an incompatible version of angular or no angular at all. It's probably not a common use-case, but an important one for us as well as some other companies I have talked to while attending ng-conf.

To fix this problem personally for our application I use a custom version of angular bundled with our application and bootstrapped manually when it's injected. The changes I made can be seen here: https://github.com/snapwich/angular.js/commit/00d26681164b47b7c556553ce39e3860528986eb

Does anyone else find disabling global export as a useful use-case (either as I did above, or in some other way), and is it something we could consider adding to the master branch?

[linclark]

I've written up some documentation on how to make use of this.

[btford]

Thanks, Lin!

On Wed, Mar 25, 2015 at 12:53 PM Lin Clark notifications@github.com wrote:

I've written up some documentation
http://blog.npmjs.org/post/114584444410/using-angulars-new-improved-browserify-support
on how to make use of this.

—
Reply to this email directly or view it on GitHub
#10732 (comment).

[micahblu]

Any plans to support AMD, most specifically requireJS?

[stryju]

@micahblu it's been "supported" for a long time, with shims.

also, you can easily load common.js modules in requirejs: http://requirejs.org/docs/api.html#packages