feat(angular.info): optionally store and access additional info on mo… #12465

petebacondarwin · 2015-07-29T22:19:27Z

…dules

This feature allows the developer to store additional meta data about a
module when it is defined by adding an additional parameter containing
an object to the angular.module(moduleName, deps, info, configFn) call.

Developers can then access this information by calling angular.info(moduleName)

See this angular/material#3842 for more background.

lgalfaso · 2015-07-29T22:44:45Z

This approach may solve the issue of getting some meta-data out of a module (if a new bootstrap is made). On the other hand, given that it is possible to have modules redefinition, there would be no mechanism to see the metadata of the instance of a module that is used in any given $injector. Is the new functionality going to be used to know before bootstrapping the information of a given module or is the information going to be used after bootstrapping to know the metadata of a module loaded in the current $injector?

petebacondarwin · 2015-07-30T18:20:10Z

The primary use case came from Angular Material, where they wanted to publish the version of the module so that when you are debugging some app, you can quickly drop into the browser console and see what versions are loaded.

petebacondarwin · 2015-07-30T18:21:15Z

@lgalfaso is it common to load a module, create an injector, override the module with a new module and then create another injector in a single application?

ThomasBurleson · 2015-07-31T03:13:32Z

test/AngularSpec.js

+      angular.module('a', [], {some: 'thing'});
+      angular.module('b', ['dep'], {other: 'thang'}, function configFn() {});
+
+      expect(info('a')).toEqual({some: 'thing'});


Where is the info() defined ?

It is defined in the closure and on angular so we could make this test more clear.

kk. The definition is in src/Angular.js.

ThomasBurleson · 2015-07-31T03:19:19Z

@petebacondarwin - The function signature

return function module(name, requires, configFnOrInfo, configFn) {
};

looks like it will present issues Angular Material using Angular versions <1.4.4; ngMaterial would have to pre-check the registration call using angular.module.length right ?

It appears that an alternative signature would not require an argument count precheck.

return function module(name, requires, configFn, info) {
};

It is late, perhaps I am not thinking clearly here.

petebacondarwin · 2015-07-31T06:28:26Z

The configFn is optional and often left out so we would need to do some wrangling either way.

lgalfaso · 2015-07-31T19:58:32Z

The configFn is optional and often left out so we would need to do some wrangling either way.

I think that the intention of the comment is that if someone wants to make a module that is 1.4.3 and 1.4.4 compatible, then, to be safe, would need to put the info parameter in the forth position anyhow. So why just not make it the forth position in the first place.

lgalfaso · 2015-07-31T20:03:27Z

@lgalfaso is it common to load a module, create an injector, override the module with a new module and then create another injector in a single application?

For apps where there are multiple non-trivial angular apps running at the same time and browserify (or any of the similar libraries are used), then the chances of having a module redefinition is quite high (even when in most cases the redefinition is just to a slightly different version of the same module (eg ui-router).

I think that this falls into the less than 5% of the cases. That said, I do not think it would be a more code to make it work as $injector.info(moduleName) (or even both ways and one would be the info on the currently loaded module and the other on the currently running in that injector)

petebacondarwin · 2015-08-04T23:38:19Z

The configFn is optional and often left out so we would need to do some wrangling either way.

I think that the intention of the comment is that if someone wants to make a module that is 1.4.3 and 1.4.4 compatible, then, to be safe, would need to put the info parameter in the forth position anyhow. So why just not make it the forth position in the first place.

@lgalfaso but both the old (configFn) and new (info) parameters are optional so it is backward compatible: you can still do:

angular.module(moduleName, depencencyArray, configFn);

I know that it is the norm to add new parameters to the end of the argument list but when both are optional then it doesn't make much difference. In this case, I believe that it is more pleasant, if you are going to provide both info and a configFn for the configFn to come at the end:

angular.module('someModule`, ['someDependency'], {version: '1.0.2'}, function() {
  ...
});

lgalfaso · 2015-08-05T10:31:02Z

@petebacondarwin I am just saying that, if people want to be safe, then they will need to put the info parameter in the forth position. The reason being that if they put it in the third position and this module is used with angular 1.4.3, then it would fail.

Note: If people have complete control of the environment their module will run, then this is not an issue, but this is an issue for material design as they should work fine with angular 1.4.3

petebacondarwin · 2015-08-05T13:55:34Z

Oh right! The concern is that people using this feature will cause their modules to be incompatible with previous versions of angular.

This would still be the case if we add it as a fourth parameter but the user chooses not to provide a configFn.

So I think we should move this out of the module function and have it as a method.

ThomasBurleson · 2015-08-05T14:56:19Z

@petebacondarwin 👍

…dules This feature allows the developer to store additional meta data about a module when it is defined by calling `mod.info(infoObject)` on the module object. Developers can then access this information by calling `mod.info()` or `angular.info(moduleName)`. See this angular/material#3842 for more background. Closes angular#12465

ThomasBurleson · 2015-10-16T18:03:58Z

src/loader.js

+           */
+          info: function(value) {
+            if (isDefined(value)) {
+              info = value;


I think we should do

info = angular.extend(info, angular.isObject(value) ? value : {});

That would mean one could add to the info but never remove anything, right?

@ThomasBurleson - I would rather not use extend here. Can you give a concrete use case?

@petebacondarwin

angular .module('ngMaterial',['ngAnimate']) .info({ version : "1.0", name : "Angular Material" }); // Later during runtime var info = angular.info().ngMaterial; info.license = "MIT";

The goal of the

info = angular.extend(info, angular.isObject(value) ? value : {});

was to require that a hashamp object was registered not just a simply value.

we could ensure that value is an object with a check for isObject, rather than using extend().

IgorMinar · 2015-11-05T16:39:48Z

What if angular.info() returned a map with all available module names as keys and an object with module specific data (or an empty object if no data is provided).

that way it's easy to see what's available without many angular.info(moduleName) calls.

ThomasBurleson · 2015-11-05T17:15:18Z

I think both would be great:

angular.info(<moduleName>) : for specific module info object
angular.info( ) : for hashmap of all modules info objects keyed by moduleName

@IgorMinar - thoughts?

IgorMinar · 2015-11-06T07:17:55Z

angular.info()[moduleName] is the same as angular.info(moduleName), so why provide both?

ThomasBurleson · 2015-11-06T11:53:07Z

@IgorMinar - Agreed: superfluous.

petebacondarwin · 2016-10-07T10:08:15Z

src/loader.js

@@ -79,6 +79,9 @@ function setupModuleLoader(window) {
     * @returns {module} new module with the {@link angular.Module} api.
     */
    return function module(name, requires, configFn) {
+
+      var info = {};


probably better to use a createMap() call here

petebacondarwin · 2016-10-07T10:09:09Z

test/AngularSpec.js

+      expect(info('b')).toEqual({other: 'thang'});
+    });
+
+    it('should return anh empty object if there is no such module', function() {


petebacondarwin · 2016-10-07T13:03:53Z

Closing in favour of #15225

petebacondarwin added type: feature frequency: low cla: yes needs: review component: misc core needs: public api severity: inconvenient 1.5-candidate labels Jul 29, 2015

petebacondarwin mentioned this pull request Jul 29, 2015

build: Publish version # for Material angular/material#3842

Closed

petebacondarwin force-pushed the angular.info branch from 71e9b84 to afc48a6 Compare July 29, 2015 22:32

caitp added this to the 1.5.x - migration-facilitation milestone Jul 30, 2015

petebacondarwin force-pushed the angular.info branch from afc48a6 to 420fc61 Compare July 30, 2015 18:28

ThomasBurleson reviewed Jul 31, 2015
View reviewed changes

petebacondarwin force-pushed the angular.info branch from 420fc61 to 51f8a57 Compare October 16, 2015 16:08

ThomasBurleson reviewed Oct 16, 2015
View reviewed changes

petebacondarwin modified the milestones: 1.6.x, 1.5.x - migration-facilitation Nov 10, 2015

Narretz force-pushed the master branch from 70049ae to b77e14b Compare January 16, 2016 21:59

petebacondarwin commented Oct 7, 2016

View reviewed changes

petebacondarwin mentioned this pull request Oct 7, 2016

Module info #15225

Closed

3 tasks

petebacondarwin closed this Oct 7, 2016

petebacondarwin deleted the angular.info branch November 24, 2016 09:04

feat(angular.info): optionally store and access additional info on mo… #12465

feat(angular.info): optionally store and access additional info on mo… #12465

Uh oh!

Conversation

petebacondarwin commented Jul 29, 2015

Uh oh!

lgalfaso commented Jul 29, 2015

Uh oh!

petebacondarwin commented Jul 30, 2015

Uh oh!

petebacondarwin commented Jul 30, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ThomasBurleson commented Jul 31, 2015

Uh oh!

petebacondarwin commented Jul 31, 2015

Uh oh!

lgalfaso commented Jul 31, 2015

Uh oh!

lgalfaso commented Jul 31, 2015

Uh oh!

petebacondarwin commented Aug 4, 2015

Uh oh!

lgalfaso commented Aug 5, 2015

Uh oh!

petebacondarwin commented Aug 5, 2015

Uh oh!

ThomasBurleson commented Aug 5, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

IgorMinar commented Nov 5, 2015

Uh oh!

ThomasBurleson commented Nov 5, 2015

Uh oh!

IgorMinar commented Nov 6, 2015

Uh oh!

ThomasBurleson commented Nov 6, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petebacondarwin commented Oct 7, 2016

Uh oh!

Uh oh!