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

New API Proposal #27

Open
michaelolof opened this issue Jun 5, 2019 · 8 comments
Open

New API Proposal #27

michaelolof opened this issue Jun 5, 2019 · 8 comments
Labels
enhancement New feature or request

Comments

@michaelolof
Copy link
Owner

A lot of the motivation behind this new proposal is making the Class API feel and work as much as natural classes as possible.

New Class Declaration API - (No Decorators)

import { initializeStore } from "vuex-class-component";

class Person extends initializeStore({ namespaced: true }) {

  // state
  firstname = "John";
  lastname = "Doe";
  age = 20;

  // mutation
  setBio = function( this :Person, bio :Bio ) {
    this.firstname = bio.firstname;
    this.lastname = bio.lastname;
    this.age = bio.age
  }

  // action
  async doAsyncStuff() {
    ...
  }

  // getters
  get fullname() {
    return this.firstname + " " + this.lastname;
  }

  // getter mutation (only allowed for getters)
  set fullname( fullname :string ) {
    const names = fullname.split( " " );
    this.firstname = names[ 0 ];
    this.lastname = names[ 1 ];
  }

}

interface Bio {
  firstname :string;
  lastname :string;
  age :number;
}

Cleaner Extraction of Vuex Modules and Proxies

import { extractVuexModule, createVuexProxy } from "vuex-class-component";
import { Person } from "./vuex/person.vuex"


export const store = new Vuex.Store({
  modules: {
    ...extractVuexModule( Person ),
  },
});

export const vxm = {
  person: createVuexProxy( Person, store ),
}

Automatic Getters an Mutations for your States = 10x More Powerful Proxies.

class StateStore extends initializeStore({ namespaced: true }) {

  one = "One";
  two = {
    deepTwo: "deep two",
    deepThree: {
      deeperThree: "deeper three"
      anotherDeeperThree: {
         ok: "Ok",
        cool: "Cool"
      }
    }
  }

}


const stateStore = createVuexProxy( StateStore, store );

stateStore.one // "One"
stateStore.one = "Changed One";
stateStore.one // Changed One

stateStore.two.deepTwo // "deep two";
stateStore.two.deepTwo = "Changed deep two";
stateStore.two.deepTwo // Changed deep two.

stateStore.two.deepThree.anotherDeeperThree.cool // "Cool";
stateStore.two.deepThree.anotherDeeperThree.cool = "Changed Cool"; // This will only mutate the cool property not the entire object.
stateStore.two.deepThree.anotherDeeperThree.cool // "Changed Cool"

What do you think?

@michaelolof michaelolof added the enhancement New feature or request label Jun 5, 2019
@asmadsen
Copy link
Contributor

asmadsen commented Jun 7, 2019

I think it's a good idea to simplify the API, and make it usable without any processing. However I'm not quite sold on the way of declaring mutations, as I don't think it should enforce a specific way of naming the method and field names.
My suggestion is that mutations should be defined as setters, ie.

class Person extends initializeStore({ namespaced: true }) {
  // [...]

  // mutation
  set Bio(bio :Bio ) {
    this.firstname = bio.firstname;
    this.lastname = bio.lastname;
    this.age = bio.age
  }
  // [...]
}

Another options is to drop mutations all together, as (I'm pretty sure) Vuex will in the future merge actions and mutations.

@michaelolof
Copy link
Owner Author

Hello @asmadsen thanks for all your work thus far.

Setters was the original idea. but the problem with setters is that their types are not predictable.

From that example, assuming a proxy was created.

const person = createProxy( Person );

console.log( person.bio ) // undefined

The user will naturally expect person.bio to return a value since bio is a property with type {firstname :string, lastname :string, age :number} but he gets undefined since the property has no corresponding getter. And there is no way of telling from the type system.
Getters on their own are safe because the type system will immediately mark any getter defined without a setter as readonly.

PS: The library will actually prevent you from using setters without getters and tell you to either define a corresponding getter or define it explicitly with a function assignment.

Also on this:

as I don't think it should enforce a specific way of naming the method and field names

The library actually doesn't enforce a particular naming convention for mutations. We could have called it mutateBio or doSomethingCrazy and it would still work as long as you're doing a function assignment instead of a function declaration

Another options is to drop mutations all together, as (I'm pretty sure) Vuex will in the future merge actions and mutations.

On this I quite agree, but I don't think they have come up with how that will work. We also can't be ahead of them. What if they decide to change course.

Also I feel with the implicit mutations you get from your proxies, you'd hardly have to reach for the mutation functions unless you want to do mass mutations of your state.

@asmadsen
Copy link
Contributor

I would agree with you when it comes to not making features that deviate from core Vuex functionality. However I think that it's better to add magic inside the actions so they act as mutations as well. Rather than relying of the difference being function assignment vs function declaration since it's not really clear why they do different things.

So in my opinion it's better to wait for Vuex to merge the concepts of actions and mutations or turn the setters on this passed to actions into small mutations.

@michaelolof
Copy link
Owner Author

Rather than relying of the difference being function assignment vs function declaration since it's not really clear why they do different thing

I understand your concern on this. I generally don't feel this is a deal breaker though.

So in my opinion it's better to wait for Vuex to merge the concepts of actions and mutations or turn the setters on this passed to actions into small mutations.

This I totally agree with. I wish Vuex will just give us a simple function that has access to the context object. We can then decide if that function will return void a promise or even a simple value. basically just like any normal function would.

@asmadsen
Copy link
Contributor

I will have to disagree with you there, because as far as I know there is rarely expected to any JS developer that function declaration and function assignment is supposed to result in different functionality (other than the classic
this binding).

And I would argue that also will be a deviation from the original Vuex interface, since it's not clear from just reading the code that the difference between the function declaration and the function assignment is a functional choice and not a style choice.

In that case I would say that it's clearer to understand that any function could manipulate the state, and there is no separation of actions and mutations.

@michaelolof
Copy link
Owner Author

michaelolof commented Jun 24, 2019

So an update on this.

I'm still very much interesting in fading out decorators from the API. Thus while th @mutation @action @Module @getter decorators will still be supported.
I won't be adding any additional decorators to the API.

The updated API proposal would look like this.

import { initializeModule, mutation } from "vuex-class-component";

const VuexModule = initializeModule({ 
  namespaced: true,
  enableLocalWatcher: true,
  enableLocalSubscriber: true,
  enableLocalSubscriberAction: true, 
})

class Person extends VuexModule {
  
  // state
  firstname = "Michael";
  lastname = "Olofinjana";
  age = 20;

  // explicit mutation
  @mutation setBio( bio :Bio ) {
    this.firstname = bio.firstname;
    this.lastname = bio.lastname;
    this.age = bio.age
  }

  // actions
  @action async fetchBio() { ... }

  @action async tryAnotherAsync { ... }

  // getters
  get fullname() {
    return this.firstname + " " + this.lastname;
  }

  // mutation for getter (only allowed for getters)
  set fullname( fullname ) {
    const names = fullname.split( " " );
    this.firstname = names[ 0 ];
    this.lastname = names[ 1 ];
  }

  // watch getters and states
  static $watch = {
    
    firstname() {
      console.log( "firstname has changed." );
    }

    fullname( this :Person, newVal :string, oldVal :string ) {
      this.age = 22;
      console.log( "new:", newVal, "old:", oldVal );
    }

  }

  // subscribe to mutations.
  static $subscribe = {

    setBio( this :Person, payload :Bio ) {
      console.log( "Set bio mutation has been called", payload );
    }

  }

  // subscribe to actions.
  static $subscribeAction = {

    fetchBio( this :Person, payload :any ) { ... }
    
    tryAnotherAsync: {
       before: ( this :Person, payload :any ) { ... },
       after: ( this :Person, payload :any ) { ... }
    }

  }


}

@bestickley
Copy link

This looks great! When will the new API be available to use?

@Velua
Copy link

Velua commented Jun 27, 2020

What happened to this? Looked good.

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

No branches or pull requests

4 participants