Dynalon / reactive-state

Redux-clone build with strict typing and RxJS down to its core. Wrist-friendly, no boilerplate or endless switch statements
MIT License
138 stars 7 forks source link

Styleguide ideas #5

Closed Ronsku closed 6 years ago

Ronsku commented 7 years ago

I thought I would share some styleguide ideas that are based on code we've been discussing before:

counter.ts: In the reducers there is a bug in TypeScript that does not check the object properties correctly when using the spread operator: https://github.com/Microsoft/TypeScript/issues/13878 . It will be fixed in TypeScript 2.4. After that the reducers will be type-safe.

import { Store, Reducer, Action } from "reactive-state";

export interface CounterState {
    value: number;
}
export const initialCounterState = {
    value: 0
}

export const increment = new Action<number>();
export const incrementReducer: Reducer<CounterState, number> = (state, payload = 1) => {
    return { ...state, value: state.value + payload };
}

export const decrement = new Action<number>();
export const decrementReducer: Reducer<CounterState, number> = (state, payload = 1) => {
    return { ...state, value: state.value - payload };
}

export const registerCounterReducers = (store: Store<CounterState>) => {
    store.addReducer(increment, incrementReducer);
    store.addReducer(decrement, decrementReducer);

        // This part I'm not super happy about, more about it later in this post.
    return {
        increment,
        decrement
    }
}

export const registerCounterSelectors = (store: Store<CounterState>) => {
    return {
        getValue: store.select(s => s.value)
    }
}

store.ts: Function names could be thought about to not be too long, also they could maybe always start with the reducer name e.g. counterRegisterReducer and counterInitialState. This would make store.ts a bit cleaner and easier to read I guess.

import { Store, Reducer, Action } from 'reactive-state';
import { Observable } from 'rxjs/Observable';
import {
  CounterState,
  initialCounterState,
  registerCounterReducers,
  registerCounterSelectors
} from './reducers/counter';

export interface AppState {
    counter: CounterState;
}

const appInitialState = {
    counter: initialCounterState
}

export const rootState = Store.create(appInitialState);

const counterStore: Store<CounterState> = rootState.createSlice('counter');
export const counterActions = registerCounterReducers(counterStore);
export const counterSelectors = registerCounterSelectors(counterStore);

app.ts or anywhere you would like to use the store: Always import Actions and Selectors from the same place. One thing that could make it easier and cleaner would be to make addReducer return the action instead of the Subscription (maybe I use it wrong?).

import { AppState, counterActions, counterSelectors } from './store';
import { Observable } from 'rxjs/Observable';

class AppComponent {
    testObservable$: Observable<number>;

    constructor() {
        this.testObservable$ = counterSelectors.getValue;
    }

    nextnumber() {
        counterActions.increment.next();
    }
}

Ideas and comments are more than welcome!

Dynalon commented 7 years ago

Thanks for pointing out the issues with the reducers, I didn't notice that. So far I agree with the style guide.

For the Actions, I tend to disagree about returning them in the registerReducer function but have them just exported as static constant for multiple reasons:

Ronsku commented 7 years ago

I tried out a couple of best practise as style-guides:

export interface LoginState { username: string; password: string; }

export const initialLoginState: LoginState = { username: undefined, password: undefined };

export class LoginReducer { private store: Store;

setUsername = new Action<string>('admin/login/SET_USERNAME'); getUsername;
setPassword = new Action<string>('admin/login/SET_PASSWORD'); getPassword;
private setUsernameReducer: Reducer<LoginState, string> = (state, username) => ({ ...state, username });
private setPasswordReducer: Reducer<LoginState, string> = (state, password) => ({ ...state, password });

constructor(store: Store<LoginState>) {
    this.store = store;

    this.store.addReducer(this.setUsername, this.setUsernameReducer);
    this.store.addReducer(this.setPassword, this.setPasswordReducer);
    this.getUsername = this.store.select(s => s.username);
    this.getPassword = this.store.select(s => s.password);
}

}

If you sacrifice some intellisense you can drop out a fair amount of boilerplate and do this:
`this.store.addReducer(this.setUsername, (state, username) => ({ ...state, username }));`
instead of this as separate parameter:
`private setUsernameReducer: Reducer<LoginState, string> = (state, username) => ({ ...state, username });`

- Without class
```typescript
import { Store, Reducer, Action } from 'reactive-state';

export interface LoginState {
    username: string;
    password: string;
}

export const initialLoginState: LoginState = {
    username: undefined,
    password: undefined
};

// This is the problem, every module would have a separate store.. not good!
export const loginStore: Store<LoginState> = Store.create(initialLoginState);

export const setUsername = new Action<string>('admin/login/SET_USERNAME');
const setUsernameReducer: Reducer<LoginState, string> = (state, username) => ({ ...state, username });
loginStore.addReducer(setUsername, setUsernameReducer);

export const setPassword = new Action<string>('admin/login/SET_PASSWORD');
const setPasswordReducer: Reducer<LoginState, string> = (state, password) => ({ ...state, password });
loginStore.addReducer(setPassword, setPasswordReducer);

export const getUsername = loginStore.select(s => s.username);
export const getPassword = loginStore.select(s => s.password);

I'm not that happy about the Class-version, but posted it here still if it in any way would give you some ideas.

I like the second version, but I'm unsure how to connect the loginStore to my rootStore to not end up with separate stores in the modules. We want to have 1 store, so I came up with this, but it's not perfect:

login.reducer.ts

import { Store, Reducer, Action } from 'reactive-state';
import { rootStore } from './../store';

export interface LoginState {
    username: string;
    password: string;
}

export const initialLoginState: LoginState = {
    username: undefined,
    password: undefined
};

// This would be nice to be able to just create.store(),
// export them to combine them somehow in store.ts.
// That would get rid of the store.ts useless boilerplate
// That way store.ts would never need modifications when adding new modules / reducer files
const loginStore: Store<LoginState> = rootStore.createSlice('login', initialLoginState);

export const setUsername = new Action<string>('admin/login/SET_USERNAME');
const setUsernameReducer: Reducer<LoginState, string> = (state, username) => ({ ...state, username });
loginStore.addReducer(setUsername, setUsernameReducer);

export const setPassword = new Action<string>('admin/login/SET_PASSWORD');
const setPasswordReducer: Reducer<LoginState, string> = (state, password) => ({ ...state, password });
loginStore.addReducer(setPassword, setPasswordReducer);

export const getUsername = loginStore.select(s => s.username);
export const getPassword = loginStore.select(s => s.password);

store.ts:

import { Store } from 'reactive-state';
import { enableDevTool } from 'reactive-state/dist/devtool.js';

import { LoginState } from './login/login.reducer';

// This is just boilerplate that has to be here... :/
// It has to be here to be able to create a slice with the string 'login'.
export interface AppState {
    login: LoginState;
}

export const rootStore: Store<AppState> = Store.create();
enableDevTool(rootStore);

Now when we would use it we would not have a "Public store API". Everything would stay in the module and you would always import actions and selectors from the modules own reducer file: import { getUsername, setUsername } from './login.reducer';

What do you think about this and how can we do the last cleanups? or do you think best practise should be something entirely different?

Dynalon commented 7 years ago

You have a flaw in your code: Your login.reducer.ts relies on rootStore when calling rootStore.createSlice<LoginState>(). That way login can never ever be considered an independent module - it is tightly coupled to your root (or sometimes called module host). But a real module whould be independent of any root! Just having the code in separate files does not make up a module; given your file layout you might think this is modular but it is not: You cannot reuse the login module in another app!

To fix this, you would do it using "Inversion of Control" pattern to really have a reusable modules:

file login.reducer.ts:

export interface LoginState {
    username: string;
    password: string;
}

export const initialLoginState: LoginState {
    username: undefined;
    password: undefined;
}

export function initLoginModule(store: Store<LoginState>) {
    // register reducers and actions here inside the function at *runtime* (not at module load time!)
    // ...
}

file store.ts:

// Store is the composition root and the only place where modules come together

import { Store } from 'reactive-state';
import { enableDevTool } from 'reactive-state/dist/devtool.js';

import { LoginState, initialLoginState, initLoginModule } from './login/login.reducer';

export interface AppState {
    login: LoginState;
}

const rootStore: Store<AppState> = Store.create();
enableDevTool(rootStore);

// boostrap module(s)
const loginSlice = rootStore.createSlice<LoginState>('login', initialLoginState);
initLoginModule(loginSlice);

Using the code above you can see: login module does not rely on anything outside of the login module itself (especially not the root store); it is thus a real, self-contained module.

If you were using React, instead of having a function initLoginModule you could also pass the "login slice" as a prop to a container component that serves as an entry point for your module. If you were using ES6 classes, you could pass the slice as an argument in the constructor etc.

One reason why I created reactive-state was that with original Redux, reducer composition works at module load time, tightly coupling modules together to a large monolith. I wanted more "configure at runtime", to be able to add/remove reducers and slices when modules are loaded and unloaded.

Dynalon commented 7 years ago

Btw. you do not have to specify the type in the rootState if you don't need it (you probably won't). And just let it up to the module to create the needed slice:

// no interface AppState required here...

// if you do not pass an argument to Store.create, the empty object {} is used as initial root state
const rootStore: Store<any> = Store.create();
enableDevTool(rootStore);

initModule(rootStore);

And your login.reducer.ts:

export function initModule(rootStore: Store<any>) {
    const loginSlice = rootStore.createSlice<LoginState>('login', initialLoginState, null);
}

Note: The third argument "null" is the cleanup state; when using .destroy() on the slice, the rootState will set the property login to that value - you would use that when unloading a module.

That way you don't have to assemble your root/app state at all with a specific type. You start with an empty object, and let the slices create the properties for you. That way you could start with a generic array of modules, and bootstrap the modules without knowing anything about them.

I personally do not like this approach; I want my rootState to be statically typed.

Ronsku commented 7 years ago

Hi, thank you again for your great answers!

I think I went a bit on a detour when I started to think about this more. I like the way you explained and it reflects a lot on the first version I was building with.

I definitely like the way of bootstrapping the modules on runtime and not on on module load, but since in this case store.ts just bootstraps the login reducer immediately, it still quite the same as on module load time. Do you think in a real life scenario that the modules should be loaded for example on routing or triggered from other actions? store.ts-could export functions that will initialise and destroy these different modules when needed/not needed?

Example:

import { Store } from 'reactive-state';
import { enableDevTool } from 'reactive-state/dist/devtool.js';

import { LoginState, initialLoginState, initLoginModule } from './login/login.reducer';

export interface AppState {
    login: LoginState;
}

const rootStore: Store<AppState> = Store.create();
enableDevTool(rootStore);

// boostrap module(s)
let loginSlice;
bootstrapLoginModule() {
    rootStore.createSlice<LoginState>('login', initialLoginState, null);
    initLoginModule(loginSlice);
}

destroyLoginModule() {
    loginSlice.destroy();
}

Thanks again and have a nice day!

Dynalon commented 7 years ago

Well for real-life scenario I assume one would use React/Angular/Vue, and in this case the registering of the modules with custom reducers/actions are most likely triggered by the component lifecycle callbacks:

In React/Angular you would have "dumb" container components for your modules, which would be the perfect place to put the reducer registration/deregistation logic. I don't know about Vue.js. Those dumb components would be created possible on a route, but also might be created upon user interaction etc.

Ronsku commented 7 years ago

I played around some more and here is what I came up with (even though this is not a complete solution):

login.reducer.ts:

import { Store, Reducer, Action } from 'reactive-state';

export interface LoginState {
    username: string;
    password: string;
}

export const initialLoginState: LoginState = {
    username: undefined,
    password: undefined
};

// I have to either here create massive amount of boilerplate here:
// export let setUsername;
// export let setPassword;
// export let getUsername;
// export let getPassword;
// (These would be assigned after initLoginReducers() would have been ran.)
// Then they could be exported/imported separately.

// Or return the actions/selectors from the init function...
export function initLoginReducers(loginSlice: Store<LoginState>) {
    const loginStore = {
        actions: {
            setUsername: new Action<string>('admin/login/SET_USERNAME'),
            setPassword: new Action<string>('admin/login/SET_PASSWORD')
        },
        selectors: {
            getUsername: loginSlice.select(s => s.username),
            getPassword: loginSlice.select(s => s.password)
        }
    };

    // Reducers
    const setUsernameReducer: Reducer<LoginState, string> = (state, username) => ({ ...state, username });
    loginSlice.addReducer(loginStore.actions.setUsername, setUsernameReducer);

    const setPasswordReducer: Reducer<LoginState, string> = (state, password) => ({ ...state, password });
    loginSlice.addReducer(loginStore.actions.setPassword, setPasswordReducer);

    return loginStore;
}

export function destroyLoginReducers(loginSlice: Store<LoginState>) {
    loginSlice.destroy();
}

store.ts:

import { Store } from 'reactive-state';
import { enableDevTool } from 'reactive-state/dist/devtool.js';

import { LoginState, initialLoginState } from './login/login.reducer';

export interface AppState {
    login: LoginState;
}

export const rootStore: Store<AppState> = Store.create();
enableDevTool(rootStore);

export const loginSlice = rootStore.createSlice<LoginState>('login', initialLoginState, null);

login.component.ts:

import { Component, OnInit, OnDestroy } from '@angular/core';

import { loginSlice } from '../store';
import { initLoginReducers, destroyLoginReducers, LoginState } from './login.reducer';

@Component({
    selector: 'g-login',
    templateUrl: './login.component.html',
    styleUrls: ['./login.component.scss']
})
export class LoginComponent implements OnInit, OnDestroy {
    loginStore = initLoginReducers(loginSlice);
    username$ = this.loginStore.selectors.getUsername;

    constructor() { }

    ngOnInit() {
        /* Could not create the public class variable top and assign it here,
        since I lose the intellisense that way.
        But I guess it's good to have it ready when we get here so we don't
        have to bother on making sure it's always on top of the function. */
    }

    ngOnDestroy() {
        destroyLoginReducers(loginSlice);
    }

    setUsername(username: string) {
        // Quite long row, but at least it's clear on what happens here.
        this.loginStore.actions.setUsername.next(username);
    }
}

login.component.html:

<div>
    <button (click)="setUsername('Foo');">Set username to Foo</button>
    <button (click)="setUsername('Bar');">Set username to Bar</button>

    <p>{{ username$ | async }}</p>
</div>

This of course is very bad if you want to access loginStore.selectors/actions from any other module, but maybe that's the point as well?

In Redux you could just pass for example loginStore.selectors.getUsername as props to your child components that need it. Wonder what would be a good way to do it in Angular..

I'm a bit afraid to build it like this, since then there would basically be separate store/module.

Biggest issue here is the exporting of actions and selectors from login.reducer.ts. How would you do it @Dynalon in this case?

Dynalon commented 7 years ago

FYI, in the meantime I have

  1. Created a react-redux like bridge that mimics the connect() function from react-redux, but is suited to reactive-state to connect a store to a component. Currently it lives in reactive-state/dist/react but will after some testing and feedback eventually moved to its own project (just like react-redux).
  2. Create a sample application with commented sourcecode as explanation how to use reactive-state as well as the react bridge described above. This could also serve as a starting point for a styleguide. The sample app uses react, react-router v4 and reactive-state.

Let me know what you think.

Dynalon commented 6 years ago

I have added a "Dos and Dont" section in the wiki: https://github.com/Dynalon/reactive-state/wiki/DosAndDonts

Along with the sample application here: https://github.com/Dynalon/reactive-state-react-example i think there is no need for an additional styleguide. I am closing this, feel free to open new issues for specific questions.

Ronsku commented 6 years ago

Awesome work! 👍 🙂