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

Functional controllers #482

Merged
merged 3 commits into from
Sep 17, 2018
Merged

Functional controllers #482

merged 3 commits into from
Sep 17, 2018

Conversation

mstijak
Copy link
Member

@mstijak mstijak commented Sep 14, 2018

This PR is introducing changes that simplify controller creation.

1. Simpler controller definition

export default {
  onInit() {
     // ...
  }
}

Note that it is not mandatory to use a class which extends the Controller base. One import statement less.

2. Functional controllers

export default ({ store}) => ({
   onInit() {
     let user = store.get("user")
   }
});

The main benefit is that this is not required for accessing the store in the controller. Knowing this upfront opens more possibilites.

2.1. Declaration of store references

This is similar to the concept explained in #347.

export default ({ store}) => {   
  const userRef = store.ref("user");
  const statusRef = store.ref("status");
  return {
    onInit() {    
      let user = userRef.value(); //store.get("user");
      statusRef.init("loading"); //store.init("status", "loading");
    
      fetch('user', data => {
        userRef.set(data); //store.set("user", data);
        statusRef.set("ok");
      });
   }
}

Another potential benefit of this approach is that references could be typed in TypeScript.
The question remains how to use references in triggers and computables.

2.2. Store method aliasing

export default ({ store }) => {   
  const { get, set, init } = store.methods();
  return {
    onInit() {    
      let user = get("user");
      init("status", "loading"); 
    
      fetch('user', data => {
        set("user", data); 
        set(status, "ok");
      });
   }
}

Would be even cooler if get, set and other methods are passed alongside store.

export default ({ get, set, init}) => ({   
  onInit() {    
    let user = get("user");
    init("status", "loading"); 
    
    fetch('user', data => {
      set("user", data); 
      set(status, "ok");
    });
});

@sasatatar
Copy link
Contributor

sasatatar commented Sep 14, 2018

This helps eliminate the use of this altogether, therefore also the need to bind methods when invoking them from another method or click handler.

  1. Functional controllers
export default ({ set, get, update }) => {
   function onInit() {
      // no need for "this.reset()" or "this.reset.bind(this)();"
      reset(); 
   }
   function increment() {
      update("counter", x => x+1);
   }
   function decrement() {
      update("counter", x => x-1);
   }
   function reset() {
      set("counter", 0);
   }
   
   return {
      increment,
      decrement,
      reset,
      onInit
   };
};

One question though, how is this.instance and this.invokeParentMethod made available when using this approach?
If these functions are just appended to the prototype, then for such cases, we could simply use this within them? That kind of defies the purpose, but still, I consider this a huge improvement :)

@mstijak mstijak merged commit dae7a9a into master Sep 17, 2018
@mstijak mstijak deleted the functional-controllers branch September 17, 2018 07:57
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

Successfully merging this pull request may close these issues.

None yet

2 participants