theKashey / memoize-state

The magic memoization for the State management. ✨🧠
MIT License
329 stars 16 forks source link

Fight the Spread #9

Open theKashey opened 6 years ago

theKashey commented 6 years ago

As continue of #5

Full object spread is a big problem - from point of view it is a performance killer, as long all object fields are set to be trackable, from another - this is why we emit a warning asking a user to reconsider selector, and that is not nice.

First version of "spread detection" just disables tracking during massive spread operation. Now - only record a fact, as long we have to emit a new "result" if any of fields got updated.

But how memoize-state tracks the keys?

Also

Missing

The idea:

What if add a special flag, to deproxify result outside of function call? Just split the calculation and the data you are going to pass throught.

Also we have to keep in mind - some spreads, are 100% "legal", as long "things just works that way". For example <Element {...props} /> - all the props are passed down to nested component.

Approach number 1

So "by result" will track the keys from "result" to be replaced by real values from "input". We still could have function memoized, but keeping "spreaded" paths updated.

const fn = state => memoize({...state, value: state.a + state.b});
// "returns" deproxifyed result {...state, value}
// should react only to .a or .b changes. But react on all props, recalculating everything.

const fn = state => memoize({...state, value: state.a + state.b}, {lazy: true});
// "returns", "raw" result {...Proxy(state), value)
// Will react only to .a or .b changes. "unwapping" proxies to the "current" values on each call.

As long object Objects and Arrays got proxies - not work for simple states. Meanwhile - could handle almost all the cases.

Approach number 2

Create a special function like Object.assign, to perform this operation without extra magic. "scanGuards" is the magic, I'll better keep of, but without them, we could not distinguish when values were "used", and when just "returned"

cons fn = state => ({...state, a: state.a, b:state.a+1});
fn({a:1, b:2});

spreaded, returned(used), used. If value was spreaded or returned - it still could be used. Only b should be omitted.

Approach number 3


const fn = state => memoizedFlow(state, state => ({value: state.a + state.b});
// will produce "expected" behavior, as long it has `state = ({...state, fn(state)})` internally, outside of memozation tracking.

But, as long the goal of this library is to be "magic" - we might things about a better way.
alex-pex commented 6 years ago

The spread detection is triggered in a case I don't understand

I have an app state with the following part :

{
  referentials: {
    clubs: {
      isLoading: false,
      ids: [
        {
          id: '/resamania/clubs/1',
          schema: 'Club'
        },
        {
          id: '/resamania/clubs/2',
          schema: 'Club'
        }
      ]
    },
    activities: {
      isLoading: true,
      ids: []
    },
    studios: {
      isLoading: true,
      ids: []
    },
    coaches: {
      isLoading: true,
      ids: []
    }
  }
}

And a selector like this

/**
 * Returns the current state of referential loading
 *
 * @param {object} state
 * @returns {boolean}
 */
export const getIsLoadingSomething = memoize(state =>
  Boolean(Object.values(getReferentials(state)).find(value => value.isLoading))
);

I'm getting

memoize-state: object spread detected in state => Boolean(Object.values(getReferentials(state)).find(value => value.isLoading)) . Keys affected: [".referentials"] . This is no-op.


What I understand is that I'm iterating over all referential state and tho I'm killing optimization, but I don't know how I can do things differently. I don't want to hardcode the list of referentials. This is killing the magic of memoize-state a bit!

I read this https://github.com/theKashey/memoize-state/issues/5#issuecomment-372809558 but still don't understand

theKashey commented 6 years ago

To say the truth - the current behaviour of spread detection is wrong - Full spreads or full enumerations are absolutely legal operations.

I am looking forward to disabling it.