Component Re-render improvements

[joellanciaux]

Griddle major version

v1

Changes proposed in this pull request

In an effort to resolve #750, I've made some updates to improve some of the diffing / shouldComponentUpdate logic in a handful of locations.

Why these changes are made

• We didn't have a shouldComponentUpdate on the base Griddle, which lead to the root node being deleted on data updates when there might only be a small change.
• GriddleConnect was running into something similar where the custom store connected component is created in render, which leads to a new component when something changes.
• Reselect is awesome, but I think we made the assumption that the memoization took the properties into account, as we wanted to hang on to the previous cellProperties value for a particular column.
  • This isn't quite true, so we needed to make a selector factory so that the selector is scoped to that column.
• ~Since we were setting custom cell value components to the value property, we were losing out on any potential render control within the custom component. I moved this logic into Cell to give a little more control to the user.~
  • ~This is a slight API change :warning:~

Are there tests?

Covered by previous tests.

[dahlbyk]

This is some great stuff!

Updated GriddleConnect to not redefine connected component every render.

My first thought when I saw this render() method is that this could probably be a functional component using withContext. Am I missing something?

Scoped selectors for cell properties.

I'm having a bit of trouble wrapping my head around this - can you talk me through it?

This is a slight API change ⚠️

SemVer-wise, might consider reverting this change but including a story to demonstrate how to build a better Cell?

[joellanciaux]

My first thought when I saw this render() method is that this could probably be a functional component using withContext. Am I missing something?

I totally might be missing something, but the thought is to preserve the existing function signature of the react-redux connect function without overwriting the store context value for any custom components that might be using the vanilla connect to grab values from their application store.

If we're using withContext to change the store context value, we'll break regular connect for anything nested inside of Griddle.

can you talk me through it?

This is explained pretty well in the reselect documentation.

More or less, reselect is only memoizing the results of the cellProperties selector for a particular set of state and props. Because we're using this selector for all of the Cell elements we're building, it's constantly destroying the memoized value as it's receiving new properties for each different Cell.

Scoping the selector's use keeps reselect's memoization meaningful.

SemVer-wise, might consider reverting this change but including a story to demonstrate how to build a better Cell?

On further review, I think we can skip this change. Phew 😅

[rdavidwu]

This is great stuff @joellanciaux @dahlbyk Do you know when this is expected to be released?

[ryanlanciaux]

This one is in npm right now but not as latest. I'm going to give it another run through and push as latest sometime this weekend :+1: