Support constant and value in chaining

[leifhanack]

Hi Brian. Thanks for ngmin.

I found a little problem using ngmin. When you use the chaining syntax and have a constant defined ngmin skips that module and don't add the bracket notation.

angular.module('myModuleName').
  constant('myConstant', 'someConstant').  // this causes ngmin to stop working
  service('myFirstService', function ($scope, myConstant) {
    // ...
  }).
  factory('myFactory', function ($scope, myConstant) {
    // ...
  }).
  service('mySecondService', function ($scope) {
    // ...
  });

Current workaround is

angular.module('myModuleName', ['myDep']).constant('myConstant', 'someConstant');

angular.module('myModuleName').
  service('myFirstService', function ($scope, myConstant) {
    // ...
  }).
  factory('myFactory', function ($scope, myConstant) {
    // ...
  }).
  service('mySecondService', function ($scope) {
    // ...
  });

Thank you, Leif

[btford]

Thanks for the great writeup, @leifhanack! I'll try to fix this when I get a moment.

[iammerrick]

@btford If you could tell me where to get started I would love to fix it.

[btford]

Thanks for the offer, @iammerrick.

First write a test that fails for this. @leifhanack's example is perfect, and you can add the test to test/chain.js. It might also be good to test chaining off of multiple .constant()s.

The code for ngmin is perhaps not the best ever written, but here's where I'd start implementing. You'll need to add another "signature" to signatures/simple.js for injectables chained off of .constant(). A "signature" is an object deep compared against the AST returned by Esprima. When the deep comparison matches, the AST is marked with a property ngModule = true in lib/mark-ast-modules.js. This check is important in how ngmin detects chained calls. Finally, you'll have to add a check here-ish to not annotate .constant()s, even though they have an ngModule = true property.

If you have any questions, let me know. Thanks again. :)

[iammerrick]

@btford So I have it supporting the service immediately after a call to constant, but if you chain a service on that again it doesn't get annotated? Can you point me in the right direction to see how that chaining works? It seems the module getting annotated gets the ngmin attribute but the one following it doesn't. Thoughts?

Thanks!

[iammerrick]

@btford Sorry forgot to link to the commit. :-) https://github.com/iammerrick/ngmin/commit/4b35fb33bb596e93080a85e1ff9043cf2598e836

[iammerrick]

Like I said, it's passing through the constants and annotating the first service, but the following gets ignored.

[iammerrick]

I am confused as to how this block https://github.com/btford/ngmin/blob/master/lib/mark-ast-modules.js#L36-L43 every finds a suitable match as all the signatures have ngModule and at this point in the code ngModule hasn't been added to anything, how does the deep-compare register it as a match and then add the ngModule in the first place?

Btw, the second service isn't properly getting it's ngModule annotation thats what I am trying to figure out now.

[iammerrick]

Ahhh looks like the real awesome starts to happen once module.js signature gets matched, which then makes the other matches possible.

[iammerrick]

Alright I got it working once I came to the realization that order matters which is going to be problematic.

angular.module('mymod', []).constant('something', 'value').controller(function() {});

Works, because the order in which the constant/value matcher get executed from the signatures file. Changing the order in the code would break without updating the order in the signatures...

angular.module('mymod', []).controller(function() {}).constant('something', 'value');

The above would break because we iterate in order of the signatures file. The only solution I can think of is to make two passes for the constant|value check because the context is really what matters here. This just seems terribly non-performant. What are your thoughts?

M

[iammerrick]

This is the current solution for the order problem which is to run over it twice, feels wrong. https://github.com/iammerrick/ngmin/commit/9fda5e9f8113c1daaa52dfbd0b813fc999cea254

[iammerrick]

Not sure how to do that fancy pull-request to issue thing: https://github.com/btford/ngmin/pull/26

[btford]

@iammerrick I'll take care of cleaning it up. Looks like it's pretty close. Currently, I make multiple passes to find all of the annotations. See #18.

Thanks!

[btford]

Landed in v0.3.3. Thanks again, @iammerrick, for your hard work. Sorry I couldn't catch your comments faster, but you seem to have figured it out yourself without too much trouble.