Native class syntax breaks invoking directive controller

[smalluban]

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
Initializing controller created with native class syntax breaks directive compilation. Problem here is with pushing bindings from directive to self initalized controller instance before constructor is invoked, but ES2015 class syntax specification forbids using constructor without "new" operator.

Codepen example: http://codepen.io/smalluban/pen/eZBeNr?editors=1012

Console output:

console_runner-ba402f0….js:1 TypeError: Class constructor Controller cannot be invoked without 'new'
    at Object.invoke (angular.js:4604)
    at extend.instance (angular.js:9855)
    at nodeLinkFn (angular.js:8927)
    at compositeLinkFn (angular.js:8226)
    at compositeLinkFn (angular.js:8229)
    at publicLinkFn (angular.js:8106)
    at angular.js:1696
    at Scope.$eval (angular.js:16820)
    at Scope.$apply (angular.js:16920)
    at bootstrapApply (angular.js:1694)

What is the expected behavior?

Pushing bindings to class constructor can not be fixed. With ES5 ordinary function everything works fine, but using class syntax is not safe if you don't use transpiler(like Babel). Only way is to deprecate it and push values after class is constructed, but I have no idea if this is possible (due to fact of using this functionality).

What is the motivation / use case for changing the behavior?

There is a lot of articles where we are encourage to use class syntax for creating controllers. But using classes is danger, because application will break if you use native classes.

In my opinion using Babel to transpile class syntax is temporary. Most modern browsers already support classes, so using Angular 1 in hybrid application could be used without transpilation.

Which versions of Angular, and which browser / OS are affected by this issue? Did this work in previous versions of Angular? Please also test with the latest stable and snapshot (https://code.angularjs.org/snapshot/) versions.

Angular: 1.5.0
Browser: Every browser where class syntax is supported: Chrome 49+, Edge 13+, Firefox 45+, Safari 9+.

[gkalpak]

It works fine for me on Chrome 49 (Win 10).
The injector (which is used for instantiating the controllers) is supposed to support native Classes (but it's not possible to "pre-assign" the bindings on this).

What browser/OS did you test this on ?

[smalluban]

Tested with Macbook Pro / OS X El Capitan 10.11.3 (15D21):

Firefox 45 throws "Error: class constructors must be invoked with |new|"
Chrome 50 (I am using beta channel) throws "TypeError: Class constructor Controller cannot be invoked without 'new'"
Safari 9.0.3 throws "Error: Cannot call a class constructor"

The injector has "isClass" method which returns false for native class. But, Do bindings should work differently using "real" classes opposite to transformed to ES5? If my application use this behavior to initialize controller properties, when I switch to native classes, it can stop working properly.

[gkalpak]

OK, so this error occurs when we fail to detect that a constructor is a Class rather than a function.
This usually happens due to bugs/deviaton from spec in browser implementations of native classes.

More specifically:

• Firefox 45:
  We are affected by this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1216630
  I.e. the string representation of class MyClass {} is function MyClass() {}, which violates the 3rd requirement for toSting() as described here
• Chrome 50/51:
  There seems to be a bug where the string representation of class MyClass {} can be either class MyClass {} (which is correct) or function class MyClass {} (which is incorrect and break the $injector).
  "Normally" the latter (incorrect) version is used by Chrome, but if you set a breakpoint and step into (or over) the $injector.invoke(...) call, then the former (correct) version is used and everything works as expected !?
• Safari 9:
  Same as with Firefox 45: class MyClass {} --> function MyClass() {}.

[smalluban]

@gkalpak What do you think about my question? Should there be a different behavior for inserting bindings to native implementation of class and ordinary function constructor? Consider this code:

class Contr {
  constructor() {
    // expression is a '&' binding
    this.data = this.expression();
  }
}
app.component(..., { controller: Contr } );

When application uses Babel, controller will be a function (ES5) and bindings will be present in constructor (injector will push them). Application works fine. But, when native class would be used, injector will know that and will not push bindings. Application breaks with TypeError - this.expression is not a function...

In my opinion, it can be confusing when application starts to fall if I turn off transforming from ES6 to ES5.

[gkalpak]

Removing the current behavior would be a big breaking change and we don't want that. Yet, accessing the properties inside the constructor is already deprecated and the recommended practice is using $onInit().
This is already documented:

Deprecation warning: although bindings for non-ES6 class controllers are currently bound to this before the controller constructor is called, this use is now deprecated. Please place initialization code that relies upon bindings inside a $onInit method on the controller, instead.

I don't think there is much more we can do, other than doing our best to properly detect native classes.

[uriva]

This is currently breaking our production app. Any updates or a temporary workaround?

[thorn0]

isClass also doesn't work for me in this plunker. I'm getting this in two versions of Chrome: 1) Chrome 50.0.2661.87 m, Windows 7 x64; 2) Chrome 50.0.2661.86 (64-bit), OS X 10.9.2.

This bug isn't stably reproduced. If I edit something seemingly unrelated in app.js, the bug may disappear (e.g. if I remove the definition of the myCompo component). I tried to debug it by using a modified version of angular.js to save the stringified function/class and the regex (/^(?:class\s|constructor\()/) used for class detection to global variables. Then I played with those saved values in the console and found out that regex.test(str) returns false. The class is stringified normally, like class {.... However, this string contains \r characters. When I removed them (str=str.replace(/\r/g,'')), the regex for some reason started to work. Also the regex works if I recreate it manually. It's actually the strangest part: two identical regexen, the saved one and the one created manually, give different results.

BTW, I also found that (class{}).toString() evaluates to 'class{}', which isn't caught by the regex.

[gkalpak]

This sounds similar to #14487. @thorn0, can you verify if the problem is stil reproducible on v52 ?