Skip to content
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

Native classes controllers not working #228

Open
rumkin opened this issue Feb 24, 2017 · 6 comments
Open

Native classes controllers not working #228

rumkin opened this issue Feb 24, 2017 · 6 comments

Comments

@rumkin
Copy link

rumkin commented Feb 24, 2017

There is an issue with native class controllers in Chrome. Controller like this:

alight.ctrl.test = class {
    constructor() {;
        this.value = 1;
    }
};

and html like this:

<div al-ctrl="test"></div>

could not be initialised due to new error:

Error in controller: test Class constructor  cannot be invoked without 'new' Object {name: "test", env: Object, scope: d, element: div} TypeError: Class constructor  cannot be invoked without 'new'
@lega911
Copy link
Owner

lega911 commented Mar 4, 2017

Wow... I didn't expect to see some bad limitations in native classes:

  1. impossible to call constructor (not class) without new, so impossible to use old way to create controller
  2. impossible use "this" in constructor before super, so Scope functions ($watch, $scan) will not work in constructor, (because they should be prepared for this)

so, probably proper way to inherit it from some class Controller

class Main extends alight.Controller {
  constructor(option) {
    super(option);
    this.name = 'Hello!';
  }
};

Also I want to try some trick with temporary "this" for scope functions, maybe it will works...

@rumkin
Copy link
Author

rumkin commented Mar 6, 2017

I think it would be proper way to instantiate controllers with new always.

@lega911
Copy link
Owner

lega911 commented Mar 6, 2017

Yes, but in this case, it's impossible to pass scope as first arguments into constructor, like it works now - broken compatibility. or "this" will not have system functions like "$watch/scan"

@rumkin
Copy link
Author

rumkin commented Mar 6, 2017

In this case the better way is to use alight.Controller. As I see Scope interface is changed in v0.14 and it is not backward compatible.

@rumkin
Copy link
Author

rumkin commented Mar 9, 2017

I think that controller could looks like that:

class Controller {
    constructor(scope, el, expr, env) {
        this.$scope = scope;
        this.$el = el;
        this.$env = env;
        this.$expr = expr;
        
        this.$init();
        
        scope.$watch('$destroy', this.$destroy.bind(this));
    }
    
    $init() {
        
    }
    
    $destroy() {
        
    }
}

@rumkin
Copy link
Author

rumkin commented Mar 13, 2017

And I think that scope should not be mixed in controller. I'd prefer to assign controller to $this scope's property.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants