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

Proposal for improved mutator API #97

Open
smikula opened this issue May 3, 2018 · 2 comments
Open

Proposal for improved mutator API #97

smikula opened this issue May 3, 2018 · 2 comments

Comments

@smikula
Copy link
Contributor

smikula commented May 3, 2018

Preface

Satchel exists because, while we liked the Flux pattern and Redux in particular, there were a few things we wanted that Redux couldn't give us.

  • Strong typing on store and actions. There have since emerged a variety of patterns to accomplish this in TypeScript.
  • Avoid immutable semantics. The immutable pattern makes for harder to comprehend code and can be more prone to bugs by inexperienced devs. Today immer could solve this for us, but we get it for free with...
  • The power and simplicity of MobX. MobX allows components to be reactive with a simple @observer decorator and is highly performant by default. Whatever the front-end of our dataflow looks like, we know we want the store itself to be observable.

Beyond those things, we really like Redux, and much of Satchel is influenced by it. These goals for this new mutator API aim to bring us closer to Redux while keeping the benefits above:

  • Mutators should define the shape of the state tree. Currently the store schema is defined separately from mutators, but we want mutators to mirror the shape of the store. If the store gets it's shape from the mutators then this will necessarily be true.
  • State should be passed into the mutators. Right now mutators access the the state by importing the store and/or one or more selectors. By injecting a subtree of the state into the mutator it's clear what the scope of the mutator is. Plus it will make the mutators easier to test by obviating the need for mocking selectors.
  • Super-strict mode. We should provide a new level of strict mode that (for debug builds only, to save on perf) enforces some best practices:
    • State cannot be modified except by the mutator that defines it.
    • References to state cannot be passed as part of an action message. If necessary, action messages should contain IDs that refer to state rather than the state itself.
  • This should be a non-breaking change. A lower priority, but it should be possible to implement this without breaking the existing Satchel APIs.

API

createMutator

The first challenge with mutators is that—because they act on observable objects—there needs to be a parent object on whose properties to act. Because reducers return a state object they can literally replace the entire state. With a little support from Satchel, we can have the best of both worlds: if a mutator returns a value then that value replaces the previous state object; if it does not return a value then we keep the same state object (which presumably has had some of its properties modified).

Creating a mutator for a simple state would look like the following. The state is simply a string, and the entire value of the state gets replaced when the mutator runs.

const mutator1 = createMutator('initial value')
    .handles(actionA, (state, action) => {
        return 'a';
    })
    .handles(actionB, (state, action) => {
        return 'b';
    });

Creating a mutator that mutates an object would look like the following. Note that nothing is returned, so the reference to the state object itself remains the same.

const mutator2 = createMutator({ someProperty: 'some value' })
    .handles(actionA, (state, action) => {
        state.someProperty = 'A';
    })
    .handles(actionB, (state, action) => {
        state.someProperty = 'B';
    });

Inferring whether to replace or mutate based on the return value feels a little loose. As an alternative, instead of handles we could have two separate APIs, e.g. mutateOn and replaceOn. I'm open to ideas for better names.

combineMutators

Mutators can be combined to build up the state of the store. (TypeScript can derive the shape of the combined mutators from the child mutators.)

const rootMutator = combineMutators({
    property1: mutator1,
    property2: mutator2
});

Effectively this creates a parent node in our state tree, so that our subtree looks like:

{
    property1: 'initial value',
    property2: {
        someProperty: 'some value'
    }
}

The combined reducer shouldn't expose handles because all the handling is done in the child reducers—except for the special case where we want the subtree itself to be null. We need a few new APIs for that.

const rootMutator = combineMutators({
        property1: mutator1,
        property2: mutator2
    })
    .nullOn(actionX)
    .nullOn(actionY)
    .definedOn(actionZ);

Satchel will make sure mutators are applied top-down, so that if actionZ is dispatched we will first define the root object and then run the child mutators which may set some properties on it.

createStore

We will have to extend createStore to create a store from a mutator. Functionally this store would be just like any current Satchel store, except that it could only be modified by one of its mutators.

const getStore = createStore('store name', rootMutator);

Testing

To test a mutator, you would call applyAction on it and pass in some fake state. (This is the same API that Satchel will use internally to dispatch actions into the mutator.)

const returnValue = mutator1.applyAction(fakeAction, fakeState);

Faking state is easy—just create a plain object in the shape that the mutator handles. Because the mutator is targetted to a very small portion of the state tree, the mock data should be trivial.

We also need a way to fake an action. This is harder since (by design) only Satchel can construct actions. We'll need to provide some sort of test utils APIs to do this.

const fakeAction = createFakeAction(actionA, { someProperty: 'X' });

Code organization

Now that mutators are tightly coupled to the state of the store, it makes sense to locate them with the store, preferably following the shape of the store. (Because mutators carry the schema there is no need to define the schema separately.)

store/
    property1/
        mutator1.ts
    property2/
        mutator2.ts
    rootMutator.ts
    getStore.ts
@mohamedmansour
Copy link

@smikula any update on pushing this PR?

One thing I wanted to do with Satchel is combining mutators together, for example, if I need to do two actions, instead of creating another action that mutates the two actions together:

componentDidMount() {
      const publisherId = this.props.match.params.publisherId;
      const accountId = this.props.match.params.accountId;
      selectPublisher(params.publisherId)
      selectAccount(params.AccountId)
}

The above calls the render function twice, it would have been nice if I can combine these two together then the store would upload together. The only way to control that is to merge these two actions into another action, but that is extra code for no reason.

Something like this could work:

componentDidMount() {
      const publisherId = this.props.match.params.publisherId;
      const accountId = this.props.match.params.accountId;
      combineMutators([
          () => selectPublisher(params.publisherId), 
          () => selectAccount(params.AccountId)
     ]);
}

So it will just call the render once.

Something similar could be for loading instead of setting Loading to false after I get data (two renders), it could be just one if we combine them together .

I know virtual dom solves this by updating the ui incrementally, but it would be nice to control when the observer is being fired.

@smikula
Copy link
Contributor Author

smikula commented Jan 29, 2019

This is something of a pet project for me, so I'm not sure when it might get merged. (Though I do have most of it implemented.)

Regardless, you can still follow this pattern perfectly well with the existing API. In fact, as a rule you should not fire multiple actions sequentially within a component for exactly the reasons you mention. I'd do something like:

componentDidMount() {
    const publisherId = this.props.match.params.publisherId;
    const accountId = this.props.match.params.accountId;
    initializeForm(publisherId, accountId);
}

Then the initializeForm action would trigger multiple mutators:

// In publisherMutators.ts
mutator(initializeForm, action => {
    // Select publisher here
});

// In accountMutators.ts
mutator(initializeForm, action => {
    // Select account here
});

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