aweary / react-copy-write

✍️ Immutable state with a mutable API
MIT License
1.79k stars 54 forks source link

Add ability to take in middleware #18

Open pksjce opened 6 years ago

pksjce commented 6 years ago

https://github.com/aweary/react-copy-write/issues/17 It looks as follows when being consumed.

import React from 'react';
import createState from 'react-copy-write';

const logger = getState => next => fn => {
  const state = getState();
  console.log('prev state', state);
  next(fn);
  console.log('next state', getState());
}

const crashReporter = getState => next => fn => {
  try {
    next(fn)
  } catch (err) {
    console.error('Caught an exception!', err)

    throw err
  }
}

const { Provider, Consumer, createMutator } = createState({
  count: 0
}, [crashReporter, logger])

const increment = createMutator(draft => draft.count = draft.count + 1);
const decrement = createMutator(draft => draft.count = draft.count - 1);

function Counter() {
  return (
    <Consumer>
      {state => (
        <div>
          <button onClick={decrement}>-</button>
          <span>{state.count}</span>
          <button onClick={increment}>+</button>
        </div>
      )}
    </Consumer>
  );
}

export default () => <Provider>
  <Counter />
</Provider>

What do you think? Need some TLC for typing and tests.

tsaiDavid commented 6 years ago

I’d love to help on this!

pksjce commented 6 years ago

I'm waiting for @aweary to review it! I feel this is good step to help testing and have some dev tools for this library.

aweary commented 6 years ago

I'm a little busy at the moment, but I'll try to review soon. At a high-level I think the middleware implementation needs to be more simple. The getState API is probably more complex than I'd want. Middleware is somewhat less valuable with react-copy-write compared to Redux, since there's no concept of an action.

If we just pass prevState and nextState to the middleware it would simplify this greatly. I understand the use case of wrapping the update in a try/catch block, but ideally I think I'd rather promote using an error boundary (and ensuring errors thrown in mutators are caught by error boundaries)

Are there any other good use cases for passing fn to middleware?

pksjce commented 6 years ago

If we just pass prevState and nextState to the middleware it would simplify this greatly.

Yeah, this was my first approach.

I'd rather promote using an error boundary

An error boundary at the top level of the app could do the same job. (ie sending the event to sentry or other service)

Are there any other good use cases for passing fn to middleware?

Not that I can think of. I mainly want some middleware to get some kind of dev tooling happen.

You would accept a simpler middleware api then?

kiliancs commented 5 years ago

IMO Immer integration should be optional and added through middleware. react-copy-write is really thin and that's great. Immer is very opinionated and that's great, but also a good case for making it optional.