Support for commonJS syntax

[jgoux]

Hi,
now that angular supports commonJS syntax (https://github.com/angular/angular.js/blob/master/CHANGELOG.md#1314-instantaneous-browserification-2015-02-24), is it possible to add it to angular-material too ?

EDIT :
I hope the syntax will also hide the internal dependencies of angular-material (ngAria and ngAnimate).
For example, right now if I want to include ngMaterial in my project, I have to do :

var angular = require('angular');
require('angular-animate');
require('angular-aria');
require('angular-material');

angular.module('foo', ['ngMaterial']);

The expected syntax with commonJS support would be :

var angular = require('angular');
var angularMaterial = require('angular-material');

angular.module('foo',[angularMaterial]);

[nkoterba]

+10

My browserify build system will finally be less 'hackish' once this is added. Very excited.

[ThomasBurleson]

@jgoux - While this feature is pending, you can use JSPM to do this now:

main.js

// Export for main.js
// Load libraries

import angular from 'angular'
import 'angular-animate'
import 'angular-messages'
import 'angular-aria'
import 'angular-material'

// Load application modules

import users from './myApp/users/UsersModule'

let modules = [ 'ng', 'ngAnimate', 'ngAria', 'ngMaterial' ]
        .concat([ `${users.name}` ]);

export default angular.module( 'my-app', modules ).run();

bootstrap.js

import angular from 'angular';
import main from 'app/main';

/**
 * Manually bootstrap the application when AngularJS and
 * the application classes have been loaded.
 */
angular
  .element( document )
  .ready( function() {

    //console.log( `starting angular( ${main.name} )` );

    var body = document.getElementsByTagName("body")[0];
    angular.bootstrap( body, [ main.name ], { strictDi: false });

  });

[pr0da]

@ThomasBurleson, I think your example works with browserify (+es6ify) too. I think @jgoux try to explain that the require/import should return the string 'ngMaterial', like in other angular modules (ngAnimate, ngAria, etc.). I think JSPM don't solve that either.

import angular from 'angular';
import ngMaterial from 'angular-material';
angular.module('app', [ngMaterial]);

[jgoux]

@ThomasBurleson I meant what @pr0da said, I'm using es6 syntax too.

[ThomasBurleson]

@jgoux - Thanks for the clarification guys.

So not only do we need future support for CommonJS packaging, but also an export of the string 'ngMaterial' for easy use in angular.module('app', [ngMaterial]);.

You realize the import or require is inconsistent if we do this?

• The import angular from 'angular'; imports an object so angular.module( ) can be used
• The import ngMaterial from 'angular-material'; imports a string.

This seems fundamentally wrong in terms of consistency. But I understand that import of angular-material is never used except to load the module(s) and register the module name.

[jgoux]

@ThomasBurleson It has been decided that angular's modules return their own name, you can see the discussion about it here : angular/angular.js#10732 (comment)

[ThomasBurleson]

Fixed in Bower Material SHA eb61de21

[jgoux]

@ThomasBurleson Thanks for the reactivity !
It remains that we still have to explicitly declare ngMaterial internal dependencies (angular-animate and angular-aria).
Shouldn't these dependencies be included by ngMaterial ?
As a library user point of view, I expect this to work :

import angular from 'angular';
import ngMaterial from 'angular-material';
angular.module('app', [ngMaterial]);

So ngMaterial should be responsible to import its dependencies :

import "angular-animate"
import "angular-aria"

What do you think ?

[ThomasBurleson]

@jgoux -

After some consideration, the Angular Material team is concerned that this enhancement is a dangerous assumption: since the Angular (angular, animate, aria, messages, etc) library imports may have path dependencies or variations and are not themselves packaged within Angular Material.

For this standard , main entry of the Angular Material package, we only focus on the angular-material module.

[jgoux]

@ThomasBurleson Why not just add these two require in the index.js ? As npm install angular-material installs ngAnimate and ngAria, I see no problem to load them from index.js.

require('angular-animate');
require('angular-aria');
require('./angular-material');
module.exports = 'ngMaterial';

I don't understand why a library using third party libraries shouldn't load them for the user.
Maybe I too used to nodeJS but it makes sense to me.
Anyway, thanks for your time, it's not a big deal.

[nkoterba]

@jgoux Agree with your suggestion.

It would be nice to do npm install angular-material and have it install all of its required dependencies (minus angular) packages under its internal node_modules folder.

This is how most other NPM packages work and ensures that if I use something like browserify ngMaterial relies on the versions of angular-animate and angular-aria that it was built against.

As it is now, I am individually installing angular-animate and angular-aria from npm and then creating a "shim" file that does exactly what @jgoux writes above:

angular-material-shim.js

require('angular-animate');
require('angular-aria');
require('angular-material); // Using browserify-shim so I can reference name vs relative path to soure

module.exports = 'ngMaterial'

When I do bower install angular-material bower installs the required dependencies: angular-aria and angular-animate.

Why should the npm/CommonJS implementation not have the same behavior?

Thanks for the responsiveness and quick turnaround!

[bathos]

@nkoterba Would it really be safe to have it install its own dependencies?

The trouble (I think) with doing that is that it creates the possibility of silently duplicating the aria and animate modules for users who might include these in their projects anyway. It would all come down to installation sequence -- if they installed angular-material first, it would install aria and animate, and then if they installed aria and animate, those two dependencies would get top-level installs as well but angular-material's require() calls would resolve to the first set while other requires() would resolve to the second. This sort of duplication matters less often in server-side work, even if it isn't ideal, but for the browser it's pretty significant that now there would be two copies of both aria and animate in your bundle.

Unless I'm wrong about how Browserify handles resolution (I'm pretty sure it just uses the same system node always uses), I think it would be a lot safer to just have users require them on their own -- or, at least, include a warning in the readme explaining that if you wish to include aria and animate as direct dependencies of your app, you must install them before angular-material.

[jgoux]

@bathos This is exactly why peerDependencies exists. More about it : http://blog.nodejs.org/2013/02/07/peer-dependencies/

[bathos]

Excellent @jgoux, I think that would be the way to go then. I was aware of peerDependencies but was under the impression that they specified optional dependencies only for some reason.

[nkoterba]

@bathos It sounds like @jgoux suggestion to use the peerDependencies may work well in this case.

However, if I'm correctly understanding your original comment, you say that the following situation could occur in your node_modules dependency tree if angular-material installed its dependencies directly:

|--angular-material@0.8.3
     |-- angular-aria@1.3.15
     |-- angular-animate@1.3.15
|-- angular-aria@1.3.10
|-- angular-animate@1.3.10

In this scenario, the user installed angular-material first which uses 1.3.15 versions. Later (or perhaps previously), the user installed angular-aria and angular-animate 1.3.10 versions.

Correct me if I am wrong, but you were saying when I browserify my app, I will get angular-aria and angular-animate bundled TWICE resulting in a much larger bundle.

But isn't that also, to some degree, the desired functionality? Yes, my bundle is bigger but it also means that angular-material will not rely on old or outdated APIs while the rest of my application won't have to worry about any new or updated APIs that may break existing functionality?

As an example, we are using a 3rd-party NPM module that uses HammerJS version 1.x. However, other parts of our app that we wrote use HammerJS version 2.x. Yes, we end up with bundling both versions 1.x and 2.x but since browserify properly isolates/injects the correct version into each javascript file that requires it, it means we can use both versions safely isolated and get the desired app functionality.

Please correct me if I'm missing something or my understanding is incorrect. :-)

[bathos]

@nkoterba, that result may be desired -- but I don't think that would really be the result in practice, because Angular itself only registers modules by name. Both would get included, but one would overwrite the other (or else be ignored -- not sure how Angular handles multiple attempts to register a module under the same name) when the client actually executes the code (I'm pretty sure -- maybe there's a way around this, but afaik the names have to be unique, so all I can think of is if all angular modules included their version numbers in their angular module names, as well as those of all services, directives, etc -- a bit unwieldy).

[nkoterba]

@bathos Ah, right! I was thinking about regular node/npm modules and forgot that Angular does DI.

In my example it works, because HammerJS is just a javascript lib and not an angular module. But yes, for anything that registers with angular.module it would be bad to duplicate.

In this case, peerDependencies sounds like the way to go!

Thanks for knocking some sense into me!

[ThomasBurleson]

Fixed with

• angular/bower-material@614726c
• angular/bower-material@9a9b47d