shouldUpdate hook? (v2.0.0-rc6)

[rstacruz]

It seems the v2 API removed the shouldUpdate hook. Is there any reason for this?

[anthonyshort]

Just haven't gotten around to doing it yet. I wanted to give components a default shouldUpdate, but it could cause some user confusion if things aren't updating like they expect. e.g. If one of their props is an object that they mutate over and over, then the shouldUpdate would see the same object each time and assume things haven't changed.

The best thing is probably to just make shouldUpdate always return true, then just have a module that includes a sane version:

import shouldUpdate from 'immutable-should-update'

export default { 
  render,
  shouldUpdate
}

[anthonyshort]

Should update should probably just look like:

function shouldUpdate (prev, next) {
  // compare .props, .children, .context in both. If props is shallow equal
 // and context and children are exactly the same, just bail. 
}

[rstacruz]

yeah, i agree, true by default should be good. having shouldUpdate overridable should make it flexible enough for those who might want to, say, use immutable.js's is() or whatnot.

[anthonyshort]

Only reason I was hesitant to add it was that you can get the same thing with memoization, which is more 'standard' than something like this.

[rstacruz]

actually, that seems like a pretty good way to accomplish it, given that there aren't any React-like "classes" in the way...

[troch]

Only reason I was hesitant to add it was that you can get the same thing with memoization, which is more 'standard' than something like this.

100% agree

[rstacruz]

I just realized memoization isn't straight-forward.

I can make a memoize() high-order function that ensures subsequent calls with the same props will not re-render, but in practice, a render() call will probably take context into effect. This isn't the problem with, say, react-redux or deku-redux because it'll take a slice of the Store and put it into props.

The way around this is to make a connect() high-order component (takes in a component, returns another component) that bakes context into props as needed, then memoize that.

module.exports = memoize(connect(map, { render }))

function render ({ props }) {
  // ...
}

function map (context) {
  return { hello: context.theSliceIWant }
}

[troch]

I think the problem with memoizing is universal apps.

[rstacruz]

https://github.com/rstacruz/deku-memoize :)

[troch]

The way you implemented it, you memoize the last rendered output of a component as opposed to memoizing the last rendered output per component instance. Below was my attempt at it:

import equals from 'is-equal-shallow';

const lastRenderedByPath = {};
const defaultShouldUpdate = (model, prevModel) => !equals(model, prevModel);

const createMemoizedRender =
    (render, shouldUpdate = defaultShouldUpdate) =>
        model => {
            const { path } = model;

            if (!lastRenderedByPath[path] || shouldUpdate(model, lastRenderedByPath[path])) {
                const rendered = render(model);
                // Not universal friendly
                lastRenderedByPath[path] = rendered;
            }

            return lastRenderedByPath[path];
        };

[rstacruz]

ah, lame, i knew i was missing something

[rstacruz]

got it, fixed it [email protected].

[rstacruz]

also, i wouldn't use is-equal-shallow as a default there. is-equal-shallow will always return false if any values are objects:

isEqualShallow({ props: {} }, { props: {} }) // => false

[troch]

Yeah in fact I meant to shallow compare props, not models ^^

const defaultShouldUpdate = (model, prevModel) => !equals(model.props, prevModel.props);

[rstacruz]

cool!

i guess this also sets the precedent that other React-ism's can be implemented as Component decorators.

• initialState (deku-memoize)
• propTypes
• DOM refs
• maybe even setState?

[ashaffer]

IMO the default shouldUpdate behavior should be an immutable shallow equal check. Deku should be opinionated about this by default, I think. Deku 2.0 is clearly designed for use with a redux or redux-like system in which you have an immutable state atom of some kind, any other use should be considered the outlier and require additional libraries/syntax imo.

EDIT: This also has the issue that any updates at all to context invalidate the entire application. Which means that any updates to any component's local state invalidate the entire tree. This seems like an inherent problem with a global context. A shouldUpdate like this:

function shouldUpdate (prev, next) {
  // compare .props, .children, .context in both. If props is shallow equal
 // and context and children are exactly the same, just bail. 
}

Will never return false on any re-render, because context changes on every render if it is the entire state atom, as all the examples indicate (and I think represents the best practice).