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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support ES6 syntax #64

Closed
jonrimmer opened this Issue Aug 7, 2014 · 40 comments

Comments

Projects
None yet
@jonrimmer

jonrimmer commented Aug 7, 2014

Using ES6 arrow functions causes parse errors in ng-annotate.

Running ng-annotate after transpilation is not possible due to Traceur removing comments, meaning explicit @ngAnnotate markers go missing.

@olov

This comment has been minimized.

Show comment
Hide comment
@olov

olov Aug 7, 2014

Owner

Thanks for filing the issue. ng-annotate will eventually most likely support ES6, opt-in or by default. I'm concerned about the parse-speed and maintenance status of Esprima Harmony but it's not blocking. It's not among the top priorities right now though.

Regarding your problems, here are some suggestions that might help out right now, or very soon at least. First of all, reference-following support is due to be released very soon and that should hopefully mean that some of your /*@ngInject*/ can go (see #57). Second, I'm thinking about providing an alternative explicit annotation mechanism that does not rely on comments. The idea is ngInject(foobar) vs /*@ngInject*/ foobar.

Owner

olov commented Aug 7, 2014

Thanks for filing the issue. ng-annotate will eventually most likely support ES6, opt-in or by default. I'm concerned about the parse-speed and maintenance status of Esprima Harmony but it's not blocking. It's not among the top priorities right now though.

Regarding your problems, here are some suggestions that might help out right now, or very soon at least. First of all, reference-following support is due to be released very soon and that should hopefully mean that some of your /*@ngInject*/ can go (see #57). Second, I'm thinking about providing an alternative explicit annotation mechanism that does not rely on comments. The idea is ngInject(foobar) vs /*@ngInject*/ foobar.

@olov

This comment has been minimized.

Show comment
Hide comment
@olov

olov Aug 7, 2014

Owner

0.9.10 is out with reference-following and ngInject(...) support. Hopefully it makes things better!

Owner

olov commented Aug 7, 2014

0.9.10 is out with reference-following and ngInject(...) support. Hopefully it makes things better!

@ghoullier

This comment has been minimized.

Show comment
Hide comment
@ghoullier

ghoullier Aug 8, 2014

There is the same problem with export keyword

export /* @ngInject */ function SomeController($scope) {
  $scope.someValue = 'Test';
}

This code produce a parse error in ng-annotate

Line 1: Unexpected reserved word

ghoullier commented Aug 8, 2014

There is the same problem with export keyword

export /* @ngInject */ function SomeController($scope) {
  $scope.someValue = 'Test';
}

This code produce a parse error in ng-annotate

Line 1: Unexpected reserved word
@olov

This comment has been minimized.

Show comment
Hide comment
@olov

olov Aug 8, 2014

Owner

Yes, it's the same for any ES6 syntax. If you must then you can workaround this by changing the Esprima dependency - replace the current entry in package.json with "esprima": "git://github.com/ariya/esprima.git#harmony", remove node_modules and then npm install. But it will slow ng-annotate down. If you do this please notice that while this will let ng-annotate process ES6 code, it won't make it understand ES6 specifics in terms of its own analysis and transformation.

Owner

olov commented Aug 8, 2014

Yes, it's the same for any ES6 syntax. If you must then you can workaround this by changing the Esprima dependency - replace the current entry in package.json with "esprima": "git://github.com/ariya/esprima.git#harmony", remove node_modules and then npm install. But it will slow ng-annotate down. If you do this please notice that while this will let ng-annotate process ES6 code, it won't make it understand ES6 specifics in terms of its own analysis and transformation.

@qraynaud

This comment has been minimized.

Show comment
Hide comment
@qraynaud

qraynaud Aug 17, 2014

@olov: I believe the harmony branch is considered very experimental by the esprima team and that they are integrating final support for ES6 in #master ATM (2.0.0-dev version). It might take them a long time to get all this working though (ES6 is a big leap for ES).

I hope it will be ready soon though because I'm also starting to work in ES6 with google-traceur on my new projects. I want to get the new syntax and features as quickly as possible.

It's sad I have to make without some of my favorite tools to get there!

Thanks for your hard work on ng-annotate! It is really appreciated ^^.

qraynaud commented Aug 17, 2014

@olov: I believe the harmony branch is considered very experimental by the esprima team and that they are integrating final support for ES6 in #master ATM (2.0.0-dev version). It might take them a long time to get all this working though (ES6 is a big leap for ES).

I hope it will be ready soon though because I'm also starting to work in ES6 with google-traceur on my new projects. I want to get the new syntax and features as quickly as possible.

It's sad I have to make without some of my favorite tools to get there!

Thanks for your hard work on ng-annotate! It is really appreciated ^^.

@olov

This comment has been minimized.

Show comment
Hide comment
@olov

olov Aug 17, 2014

Owner

Aww thanks. :-)

So we could include two versions of Esprima and let users opt-in to the slow Harmony one with a switch. Before that, I'd like to spend some time with Acorn which got some ES6 support recently as far as I know.

Owner

olov commented Aug 17, 2014

Aww thanks. :-)

So we could include two versions of Esprima and let users opt-in to the slow Harmony one with a switch. Before that, I'd like to spend some time with Acorn which got some ES6 support recently as far as I know.

@olov olov added the enhancement label Sep 4, 2014

@olov olov closed this in 5465a64 Oct 9, 2014

@olov

This comment has been minimized.

Show comment
Hide comment
@olov

olov Oct 9, 2014

Owner

I just released 0.10.2 with experimental opt-in support for the Acorn parser in ES6 mode using the --es6 switch (or es6 config option when used as an API). While this will still be slower than the default (ES5 w/ Esprima) it's faster than ES6 w/ Esprima. I don't know how complete the ES6 support is in Acorn at the moment but it seems to be actively maintained based on a quick glance at the commit history. Please let me know how this works!

Owner

olov commented Oct 9, 2014

I just released 0.10.2 with experimental opt-in support for the Acorn parser in ES6 mode using the --es6 switch (or es6 config option when used as an API). While this will still be slower than the default (ES5 w/ Esprima) it's faster than ES6 w/ Esprima. I don't know how complete the ES6 support is in Acorn at the moment but it seems to be actively maintained based on a quick glance at the commit history. Please let me know how this works!

@andreasgrimm

This comment has been minimized.

Show comment
Hide comment
@andreasgrimm

andreasgrimm Nov 3, 2014

(a slightly modified version of) @ghoullie 's example doesn't work in our case:

/* @ngInject */
export var controller = function ( $scope, $state, ...

or

export var controller = /* @ngInject */ function ( $scope, $state, ...

It doesn't throw an error with es6 : true. but it won't add the inject code either.

I guess it's because of what @olov said "If you do this please notice that while this will let ng-annotate process ES6 code, it won't make it understand ES6 specifics in terms of its own analysis and transformation" !?

andreasgrimm commented Nov 3, 2014

(a slightly modified version of) @ghoullie 's example doesn't work in our case:

/* @ngInject */
export var controller = function ( $scope, $state, ...

or

export var controller = /* @ngInject */ function ( $scope, $state, ...

It doesn't throw an error with es6 : true. but it won't add the inject code either.

I guess it's because of what @olov said "If you do this please notice that while this will let ng-annotate process ES6 code, it won't make it understand ES6 specifics in terms of its own analysis and transformation" !?

@olov

This comment has been minimized.

Show comment
Hide comment
@olov

olov Nov 3, 2014

Owner

@andreasgrimm no error you say? I wonder if you're using an old version of ng-annotate that does not understand the --es6 switch then. I tried your first example now with 0.10.3 and it crashes ng-annotate (or rather the ordered-ast-traverse module) hard. While I can fix the crash short-term, actually implementing proper es6 support is a big larger effort.

Can you consider running ng-annotate on the Traceur output meanwhile? Traceur may remove comments but you can use the function form of ngInject instead, like so: export var controller = ngInject(function $scope, $state) {..}).

Owner

olov commented Nov 3, 2014

@andreasgrimm no error you say? I wonder if you're using an old version of ng-annotate that does not understand the --es6 switch then. I tried your first example now with 0.10.3 and it crashes ng-annotate (or rather the ordered-ast-traverse module) hard. While I can fix the crash short-term, actually implementing proper es6 support is a big larger effort.

Can you consider running ng-annotate on the Traceur output meanwhile? Traceur may remove comments but you can use the function form of ngInject instead, like so: export var controller = ngInject(function $scope, $state) {..}).

@mithun-daa

This comment has been minimized.

Show comment
Hide comment
@mithun-daa

mithun-daa Nov 3, 2014

@olov How will the ngInject function form work with classes?

export class MainCtrl {
    constructor($http)
    {
        this._$http = $http;
    }
    get name() {
        return "World$%#";
    }
}

Do I put the constructor inside the ngInject function? I bet Traceur will freak out if I did.

mithun-daa commented Nov 3, 2014

@olov How will the ngInject function form work with classes?

export class MainCtrl {
    constructor($http)
    {
        this._$http = $http;
    }
    get name() {
        return "World$%#";
    }
}

Do I put the constructor inside the ngInject function? I bet Traceur will freak out if I did.

@olov

This comment has been minimized.

Show comment
Hide comment
@olov

olov Nov 3, 2014

Owner

@mithun-daa it does not. ngInject() is just a funcall and can be used where such are allowed.

Owner

olov commented Nov 3, 2014

@mithun-daa it does not. ngInject() is just a funcall and can be used where such are allowed.

@mithun-daa

This comment has been minimized.

Show comment
Hide comment
@mithun-daa

mithun-daa Nov 3, 2014

you mean this will work?

 ngInject(constructor($http)
    {
        this._$http = $http;
    });

mithun-daa commented Nov 3, 2014

you mean this will work?

 ngInject(constructor($http)
    {
        this._$http = $http;
    });
@olov

This comment has been minimized.

Show comment
Hide comment
@olov

olov Nov 3, 2014

Owner

@mithun-daa no I mean it won't, because that's not valid ES6. Please do not interpret my advice to @andreasgrimm for a specific example as something wider. ng-annotate does not yet support ES6 input properly.

Owner

olov commented Nov 3, 2014

@mithun-daa no I mean it won't, because that's not valid ES6. Please do not interpret my advice to @andreasgrimm for a specific example as something wider. ng-annotate does not yet support ES6 input properly.

@mithun-daa

This comment has been minimized.

Show comment
Hide comment
@mithun-daa

mithun-daa Nov 3, 2014

ok. Any advise in the meantime? go with functions instead of classes?

mithun-daa commented Nov 3, 2014

ok. Any advise in the meantime? go with functions instead of classes?

@olov olov reopened this Nov 3, 2014

@olov

This comment has been minimized.

Show comment
Hide comment
@olov

olov Nov 3, 2014

Owner

The advice right now is to try to use ng-annotate on the Traceur output. When explicit annotations are desired, ngInject() can be used only in places where the ES6 language allows it. /*ngInject*/ can't be used unless Traceur is modified to preserve comments.

Good news on the ES6/Traceur front is that I just released ng-annotate 0.11 with a fix for #84. Feel free to try it out and to open new issues with minimal input/output examples.

Owner

olov commented Nov 3, 2014

The advice right now is to try to use ng-annotate on the Traceur output. When explicit annotations are desired, ngInject() can be used only in places where the ES6 language allows it. /*ngInject*/ can't be used unless Traceur is modified to preserve comments.

Good news on the ES6/Traceur front is that I just released ng-annotate 0.11 with a fix for #84. Feel free to try it out and to open new issues with minimal input/output examples.

@mithun-daa

This comment has been minimized.

Show comment
Hide comment
@mithun-daa

mithun-daa Nov 4, 2014

I am running ng-annotate (tried cli and grunt task) on the output of a traceur command and nothing seems to happen. I got the latest version of ng-annotate too (v 0.11.0).
Here is my ES6 file:

export class MainCtrl {
    constructor($http)
    {
        this._$http = $http;
    }
    get name() {
        return "World$%#";
    }
}

which gets transpiled to:

System.register("src/app_temp", [], function() {
  "use strict";
  var __moduleName = "src/app_temp";
  var MainCtrl = function MainCtrl($http) {
    this._$http = $http;
  };
  ($traceurRuntime.createClass)(MainCtrl, {get name() {
      return "World$%#";
    }}, {});
  return {get MainCtrl() {
      return MainCtrl;
    }};
});

The output of ng-annotate is the exact same file as the transpiled file.. No annotations added. I was hoping to see $http being injected.

My command looks like this:

ng-annotate -a es5\temp.js -o temp2.js

Am I missing something?

mithun-daa commented Nov 4, 2014

I am running ng-annotate (tried cli and grunt task) on the output of a traceur command and nothing seems to happen. I got the latest version of ng-annotate too (v 0.11.0).
Here is my ES6 file:

export class MainCtrl {
    constructor($http)
    {
        this._$http = $http;
    }
    get name() {
        return "World$%#";
    }
}

which gets transpiled to:

System.register("src/app_temp", [], function() {
  "use strict";
  var __moduleName = "src/app_temp";
  var MainCtrl = function MainCtrl($http) {
    this._$http = $http;
  };
  ($traceurRuntime.createClass)(MainCtrl, {get name() {
      return "World$%#";
    }}, {});
  return {get MainCtrl() {
      return MainCtrl;
    }};
});

The output of ng-annotate is the exact same file as the transpiled file.. No annotations added. I was hoping to see $http being injected.

My command looks like this:

ng-annotate -a es5\temp.js -o temp2.js

Am I missing something?

@qraynaud

This comment has been minimized.

Show comment
Hide comment
@qraynaud

qraynaud Nov 4, 2014

Yeah, you still need to use injection at places where it is supported… You could do something like :

class MainCtrl {
    constructor($http)
    {
        this._$http = $http;
    }
    get name() {
        return "World$%#";
    }
}

export ngInject(function($http) {
  return new MainCtrl($http);
});

qraynaud commented Nov 4, 2014

Yeah, you still need to use injection at places where it is supported… You could do something like :

class MainCtrl {
    constructor($http)
    {
        this._$http = $http;
    }
    get name() {
        return "World$%#";
    }
}

export ngInject(function($http) {
  return new MainCtrl($http);
});
@olov

This comment has been minimized.

Show comment
Hide comment
@olov

olov Nov 4, 2014

Owner

@mithun-daa ng-annotate has no way of knowing from that snippet alone that your class is related to angular and should be annotated. Add angular.module('myApp').controller("MainCtrl", MainCtrl); and things will work better thanks to our recent reference-following support. [You'll most likely want to concatenate together your application .js files and then run ng-annotate on them in one go.]

Owner

olov commented Nov 4, 2014

@mithun-daa ng-annotate has no way of knowing from that snippet alone that your class is related to angular and should be annotated. Add angular.module('myApp').controller("MainCtrl", MainCtrl); and things will work better thanks to our recent reference-following support. [You'll most likely want to concatenate together your application .js files and then run ng-annotate on them in one go.]

@qraynaud

This comment has been minimized.

Show comment
Hide comment
@qraynaud

qraynaud Nov 4, 2014

@olov, even if he concatenates files, since he is using modules it won't work… The syntax that traceur will replace the import with won't allow ng-annotate to find the proper object to annotate… It will relsolve to a func call like :

angular.module('myApp').controller("MainCtrl", System.get('MainCtrl'));

No way to retrace it back to :

System.register('MainCtrl', function() {
  //
  return MainCtrl;
});

You need an actual ES6 parser to stand a chance to find the correct reference…

qraynaud commented Nov 4, 2014

@olov, even if he concatenates files, since he is using modules it won't work… The syntax that traceur will replace the import with won't allow ng-annotate to find the proper object to annotate… It will relsolve to a func call like :

angular.module('myApp').controller("MainCtrl", System.get('MainCtrl'));

No way to retrace it back to :

System.register('MainCtrl', function() {
  //
  return MainCtrl;
});

You need an actual ES6 parser to stand a chance to find the correct reference…

@olov

This comment has been minimized.

Show comment
Hide comment
@olov

olov Nov 4, 2014

Owner

yah, we're back to that. The parser is already there, what needs to be done is

  1. making the walker traverse the ES6 tree properly by updating https://github.com/olov/ordered-esprima-props/blob/master/ordered-esprima-props.js and
  2. updating scope analysis in ng-annotate

1 is quite suitable for contributions. Any takers?

Owner

olov commented Nov 4, 2014

yah, we're back to that. The parser is already there, what needs to be done is

  1. making the walker traverse the ES6 tree properly by updating https://github.com/olov/ordered-esprima-props/blob/master/ordered-esprima-props.js and
  2. updating scope analysis in ng-annotate

1 is quite suitable for contributions. Any takers?

@andreasgrimm

This comment has been minimized.

Show comment
Hide comment
@andreasgrimm

andreasgrimm Nov 4, 2014

exporting a (controller) function instead of exporting a class worked for
me with version 0.10.3. I ran it against the individual files of the output
of jspm build (which uses traceur under the hood), minifying and jspm
bundling afterwards

On Tuesday, November 4, 2014, Mithun Patel notifications@github.com wrote:

I am running ng-annotate (tried cli and grunt task) on the output of a
traceur command and nothing seems to happen. I got the latest version of
ng-annotate too (v 0.11.0).
Here is my ES6 file:

export class MainCtrl {
constructor($http)
{
this._$http = $http;
}
get name() {
return "World$%#";
}
}

which gets transpiled to:

System.register("src/app_temp", [], function() {
"use strict";
var __moduleName = "src/app_temp";
var MainCtrl = function MainCtrl($http) {
this._$http = $http;
};
($traceurRuntime.createClass)(MainCtrl, {get name() {
return "World$%#";
}}, {});
return {get MainCtrl() {
return MainCtrl;
}};
});

The output of ng-annotate is the exact same file as the transpiled file..
No annotations added. I was hoping to see $http being injected.

Am I missing something?


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

andreasgrimm commented Nov 4, 2014

exporting a (controller) function instead of exporting a class worked for
me with version 0.10.3. I ran it against the individual files of the output
of jspm build (which uses traceur under the hood), minifying and jspm
bundling afterwards

On Tuesday, November 4, 2014, Mithun Patel notifications@github.com wrote:

I am running ng-annotate (tried cli and grunt task) on the output of a
traceur command and nothing seems to happen. I got the latest version of
ng-annotate too (v 0.11.0).
Here is my ES6 file:

export class MainCtrl {
constructor($http)
{
this._$http = $http;
}
get name() {
return "World$%#";
}
}

which gets transpiled to:

System.register("src/app_temp", [], function() {
"use strict";
var __moduleName = "src/app_temp";
var MainCtrl = function MainCtrl($http) {
this._$http = $http;
};
($traceurRuntime.createClass)(MainCtrl, {get name() {
return "World$%#";
}}, {});
return {get MainCtrl() {
return MainCtrl;
}};
});

The output of ng-annotate is the exact same file as the transpiled file..
No annotations added. I was hoping to see $http being injected.

Am I missing something?


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

@olov

This comment has been minimized.

Show comment
Hide comment
@olov

olov Jan 1, 2015

Owner

Please check out the proposal in #124 (comment) where a "use strict"-like "ngInject" string literal would be used for ES6 constructors. Feedback welcome in #124 or here!

Owner

olov commented Jan 1, 2015

Please check out the proposal in #124 (comment) where a "use strict"-like "ngInject" string literal would be used for ES6 constructors. Feedback welcome in #124 or here!

@rootical

This comment has been minimized.

Show comment
Hide comment
@rootical

rootical Mar 2, 2015

Thanks for you work! Sorry if it's wrong place to ask.
Spent a few hours trying to make ng-annotate working with es6ify (traceur) output. Without success. I have the latest version of ng-annotate.
Please provide correct syntax to use it with es6 classes and the real state of it – searching through various issues didn't leave any clear vision of how to do that.

rootical commented Mar 2, 2015

Thanks for you work! Sorry if it's wrong place to ask.
Spent a few hours trying to make ng-annotate working with es6ify (traceur) output. Without success. I have the latest version of ng-annotate.
Please provide correct syntax to use it with es6 classes and the real state of it – searching through various issues didn't leave any clear vision of how to do that.

@olov

This comment has been minimized.

Show comment
Hide comment
@olov

olov Mar 2, 2015

Owner

@rootical use "ngInject"; inside of ES6 functions that you want to have annotated. The ES5-output from Babel will preserve these strings and you can then run them through ng-annotate. I'll add a section about this to the README eventually.

Owner

olov commented Mar 2, 2015

@rootical use "ngInject"; inside of ES6 functions that you want to have annotated. The ES5-output from Babel will preserve these strings and you can then run them through ng-annotate. I'll add a section about this to the README eventually.

@mithun-daa

This comment has been minimized.

Show comment
Hide comment
@mithun-daa

mithun-daa Mar 2, 2015

@olov That only works on functions right? not classes?

mithun-daa commented Mar 2, 2015

@olov That only works on functions right? not classes?

@olov

This comment has been minimized.

Show comment
Hide comment
@olov

olov Mar 2, 2015

Owner

@mithun-daa For classes: class MyCtrl { constructor($a, $b) { "ngInject"; ... } }

Owner

olov commented Mar 2, 2015

@mithun-daa For classes: class MyCtrl { constructor($a, $b) { "ngInject"; ... } }

@jonrimmer

This comment has been minimized.

Show comment
Hide comment
@jonrimmer

jonrimmer Mar 2, 2015

Babel will actually preserve comments as well, as long as you have the "compact" option set to "false". I am using /* @ngInject */ annotations on all my ES6 controller functions and classes.

Unless you have very good reasons to keep using Traceur, I recommend switching to Babel. It's has far better support, documentation and configurability.

jonrimmer commented Mar 2, 2015

Babel will actually preserve comments as well, as long as you have the "compact" option set to "false". I am using /* @ngInject */ annotations on all my ES6 controller functions and classes.

Unless you have very good reasons to keep using Traceur, I recommend switching to Babel. It's has far better support, documentation and configurability.

@mithun-daa

This comment has been minimized.

Show comment
Hide comment
@mithun-daa

mithun-daa commented Mar 2, 2015

👍

@rootical

This comment has been minimized.

Show comment
Hide comment
@rootical

rootical Mar 9, 2015

@olov , @jonrimmer guys, please clarify it a bit. I was able to successfully ng-annotate classes with traceur using this way class MyCtrl { constructor($a, $b) { "ngInject"; ... } }.

After switching to babel, I try to use it along with browserify-ngannotate in a gulp task basically the same way as before with traceur:

  bro = browserify('./client/app/app.js', {debug: true});
  bro.transform(babelify.configure({
    compact: false,
    comments: true
  }));
  bro.transform(ngAnnotate);

Nothing seems to be ng-annotated. I've also tried to do /* @ngInject */ instead of "ngInject"; with no luck. Am I missing something?

rootical commented Mar 9, 2015

@olov , @jonrimmer guys, please clarify it a bit. I was able to successfully ng-annotate classes with traceur using this way class MyCtrl { constructor($a, $b) { "ngInject"; ... } }.

After switching to babel, I try to use it along with browserify-ngannotate in a gulp task basically the same way as before with traceur:

  bro = browserify('./client/app/app.js', {debug: true});
  bro.transform(babelify.configure({
    compact: false,
    comments: true
  }));
  bro.transform(ngAnnotate);

Nothing seems to be ng-annotated. I've also tried to do /* @ngInject */ instead of "ngInject"; with no luck. Am I missing something?

@rootical

This comment has been minimized.

Show comment
Hide comment
@rootical

rootical Mar 9, 2015

Duck debugging helped, that's how it works with babel:

/* @ngInject */
class MyCtrl { constructor($a, $b) { ... } }

Thanks

rootical commented Mar 9, 2015

Duck debugging helped, that's how it works with babel:

/* @ngInject */
class MyCtrl { constructor($a, $b) { ... } }

Thanks

@olov

This comment has been minimized.

Show comment
Hide comment
@olov

olov Mar 9, 2015

Owner

@rootical thanks for reporting, "ngInject"; not working turns out to be a bug in Babel, which doesn't preserve directive prologues (see https://babeljs.io/repl/#?experimental=true&playground=false&evaluate=true&loose=false&spec=false&code=class%20MyCtrl%20%7B%20constructor(a)%20%7B%20%22use%20strict%22%3B%20foo%20%7D%20%7D). I'm filing a bug!

Owner

olov commented Mar 9, 2015

@rootical thanks for reporting, "ngInject"; not working turns out to be a bug in Babel, which doesn't preserve directive prologues (see https://babeljs.io/repl/#?experimental=true&playground=false&evaluate=true&loose=false&spec=false&code=class%20MyCtrl%20%7B%20constructor(a)%20%7B%20%22use%20strict%22%3B%20foo%20%7D%20%7D). I'm filing a bug!

@olov olov added L and removed 1.0 labels Mar 12, 2015

@olov

This comment has been minimized.

Show comment
Hide comment
@olov

olov Mar 12, 2015

Owner

FYI, Babel 4.7.4 and newer supports directive prologues properly so "ngInject"; will work fine with it

Owner

olov commented Mar 12, 2015

FYI, Babel 4.7.4 and newer supports directive prologues properly so "ngInject"; will work fine with it

@olov

This comment has been minimized.

Show comment
Hide comment
@olov

olov Mar 12, 2015

Owner

I'm about to release ng-annotate 1.0 soon and with it I'll close this issue. I'll add proper documentation to the README of how to deal with ES6 and TypeScript, i.e. run ng-annotate on the transpiled output and use "ngInject"; (preferred) or /*@ngInject*/ (more fragile to transpiler changes). ng-annotate supporting ES6 (ESxy) code directly could happen in the future if a contributor stepping up.

Owner

olov commented Mar 12, 2015

I'm about to release ng-annotate 1.0 soon and with it I'll close this issue. I'll add proper documentation to the README of how to deal with ES6 and TypeScript, i.e. run ng-annotate on the transpiled output and use "ngInject"; (preferred) or /*@ngInject*/ (more fragile to transpiler changes). ng-annotate supporting ES6 (ESxy) code directly could happen in the future if a contributor stepping up.

@natefaubion

This comment has been minimized.

Show comment
Hide comment
@natefaubion

natefaubion May 15, 2015

A couple of other points wrt to using ngAnnotate on the output of Babel ES6 modules. Babel will transpile an Angular import like so:

import angular from 'angular'
angular.module(...)
var _angular = require('angular');
var _angular2 = _interopRequireDefault(_angular);
_angular2['default'].module(...)

Which breaks all the usual auto-detection. Would it be possible to configure detection for the angular.module as well?

natefaubion commented May 15, 2015

A couple of other points wrt to using ngAnnotate on the output of Babel ES6 modules. Babel will transpile an Angular import like so:

import angular from 'angular'
angular.module(...)
var _angular = require('angular');
var _angular2 = _interopRequireDefault(_angular);
_angular2['default'].module(...)

Which breaks all the usual auto-detection. Would it be possible to configure detection for the angular.module as well?

@olov

This comment has been minimized.

Show comment
Hide comment
@olov

olov May 15, 2015

Owner

Check out our regexp option https://github.com/olov/ng-annotate#declaration-forms. You case use that to match [regexp].controller and so on. The default regexp is ^[a-zA-Z0-9_\$\.\s]+$ which matches things like myMod or myMod.foo.

As an example (from the README), --regexp "^require(.*)$" will work properly for require('app-module').controller(..). In your case, perhaps try --regexp "angular.*\.module(.*)$" (not tested).

Owner

olov commented May 15, 2015

Check out our regexp option https://github.com/olov/ng-annotate#declaration-forms. You case use that to match [regexp].controller and so on. The default regexp is ^[a-zA-Z0-9_\$\.\s]+$ which matches things like myMod or myMod.foo.

As an example (from the README), --regexp "^require(.*)$" will work properly for require('app-module').controller(..). In your case, perhaps try --regexp "angular.*\.module(.*)$" (not tested).

@olov

This comment has been minimized.

Show comment
Hide comment
@olov

olov May 27, 2015

Owner

1.0.0 is out so closing this because #64 (comment)

Owner

olov commented May 27, 2015

1.0.0 is out so closing this because #64 (comment)

@olov olov closed this May 27, 2015

@brettstack

This comment has been minimized.

Show comment
Hide comment
@brettstack

brettstack Jun 12, 2015

@natefaubion what did you end up doing? You should be able to call ng-annotate before babel.

brettstack commented Jun 12, 2015

@natefaubion what did you end up doing? You should be able to call ng-annotate before babel.

@natefaubion

This comment has been minimized.

Show comment
Hide comment
@natefaubion

natefaubion Jun 12, 2015

@breandr I ended up not importing angular directly (but a util lib instead), and then writing a plugin to match the (0, foo.bar)(...) syntax babel emits for imports. I had to fiddle around with $chained and $methodName (which ngAnnotate uses internally to identify nodes) to get it to work though. The plugin API could use a little love, but maybe I'm not using it correctly.

function NgAnnotateBabelPlugin() {
  var ngProps = ['directive', 'controller', 'config', 'run', 'provider',
                 'service', 'factory', 'filter', 'animation']

  this.init = _.noop
  this.match = function(node) {
    if (node.type === 'CallExpression' &&
        node.callee.type === 'SequenceExpression' &&
        node.callee.expressions[0].type === 'Literal' &&
        node.callee.expressions[0].value === 0 &&
        node.callee.expressions[1].type === 'MemberExpression' &&
        _.contains(ngProps, node.callee.expressions[1].property.name)) {

      return node.arguments.filter(function(arg) {
        if (arg.type === 'FunctionExpression') {
          arg.$chained = 4 // Regular chain
          arg.$methodName = node.callee.expressions[1].property.name
          return true
        }
      })
    }
  }
}
import { directive } from '...'

export default directive('foo', () => { ... })

natefaubion commented Jun 12, 2015

@breandr I ended up not importing angular directly (but a util lib instead), and then writing a plugin to match the (0, foo.bar)(...) syntax babel emits for imports. I had to fiddle around with $chained and $methodName (which ngAnnotate uses internally to identify nodes) to get it to work though. The plugin API could use a little love, but maybe I'm not using it correctly.

function NgAnnotateBabelPlugin() {
  var ngProps = ['directive', 'controller', 'config', 'run', 'provider',
                 'service', 'factory', 'filter', 'animation']

  this.init = _.noop
  this.match = function(node) {
    if (node.type === 'CallExpression' &&
        node.callee.type === 'SequenceExpression' &&
        node.callee.expressions[0].type === 'Literal' &&
        node.callee.expressions[0].value === 0 &&
        node.callee.expressions[1].type === 'MemberExpression' &&
        _.contains(ngProps, node.callee.expressions[1].property.name)) {

      return node.arguments.filter(function(arg) {
        if (arg.type === 'FunctionExpression') {
          arg.$chained = 4 // Regular chain
          arg.$methodName = node.callee.expressions[1].property.name
          return true
        }
      })
    }
  }
}
import { directive } from '...'

export default directive('foo', () => { ... })
@pmowrer

This comment has been minimized.

Show comment
Hide comment
@pmowrer

pmowrer Sep 3, 2015

It's a bit ironic that one of Babel's selling points is clearer transpiled source, yet it doesn't work out of the box with ng-annotate, unlike Traceur.

@olov Thanks for the regex, that works (at least for our purposes)! I've modified it the following way for safety: angular.*?\.module\(.*?\)$

pmowrer commented Sep 3, 2015

It's a bit ironic that one of Babel's selling points is clearer transpiled source, yet it doesn't work out of the box with ng-annotate, unlike Traceur.

@olov Thanks for the regex, that works (at least for our purposes)! I've modified it the following way for safety: angular.*?\.module\(.*?\)$

@e-cloud

This comment has been minimized.

Show comment
Hide comment
@e-cloud

e-cloud Oct 18, 2016

@pmowrer just find out, here is a more reasonable one: angular.*?\\.module(.*?)$

e-cloud commented Oct 18, 2016

@pmowrer just find out, here is a more reasonable one: angular.*?\\.module(.*?)$

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment