Nesting: Fix broken nested content caused by overly aggressive cache

[aduth]

Regression introduced in #5002 (1ce8079)

This pull request seeks to resolve an issue where the content within a nested block is not saved. The issue is that because the selector for getBlock was configured to cache with the individual block as a dependant, the cache is incorrectly not bust when any of its inner blocks change.

Since 1ce8079 was introduced as a performance enhancement, we should separately seek to explore how we tailor the cache to be most specific to the individual block being queried (trigger change on parent when inner blocks change?)

Testing instructions:

1. Create a Columns block
2. Insert content in one or both columns
3. Press Save
4. Refresh the post
5. Verify content is saved correctly

Follow-up Tasks:

• End-to-end test including nested blocks

[youknowriad]

Don't you think the fix should be to add a dependency to blockOrder[uid] instead? I'm afraid this means all block rerenders if only one of them changes.

[aduth]

Don't you think the fix should be to add a dependency to blockOrder[uid] instead?

I don't think this would help, as it still needs to reflect changes to inner blocks even when the order hasn't changed.

I'm afraid this means all block rerenders if only one of them changes.

Yes, this seems quite bad.

I'm going to look into an alternative approach where changing an inner block also signals a change to its root block, and perhaps refactor the child -> root referencing in the process.

[youknowriad]

I'm going to look into an alternative approach where changing an inner block also signals a change to its root block, and perhaps refactor the child -> root referencing in the process.

I'm thinking we could revisit embedding the child blocks in the "block" object. and replace them with their UIDs instead.

And if we want access to the innerBlocks (which should be very localized), we use another dedicated selector.

What do you think?

[aduth]

It seems convenient from a purely performance-oriented perspective, but not sure I'd agree that the canonical block object produced by a derived selector shouldn't also include its full children set.

[aduth]

Aside: We should probably move the attributes reducing for meta into a separate function memoized only on meta state properties. Avoids iterating unnecessarily since these rarely change.

[aduth]

A couple (very rough) iterations on ideas:

1. Create a separate memoized selector which caches on dependants, taking advantage of the behavior of createSelector's second argument to recursively create cached references, and returning the minimal result necessary to flag a new reference value (empty array).
   • https://gist.github.com/aduth/7d03f967f04338632bb0248fdb5f43f6
2. Signal a change to an inner block as a change to its root block. This is made difficult by the fact that there are many actions which change a block, so might need to be generalized to some before/after comparison of blocks (in this example, via Redux middleware)
   • https://gist.github.com/aduth/08fde265f416a00c79f5ce5142636c6b

[aduth]

I proceeded with the first of the two above explored options in b8da050.

It's a very opaque implementation, but hopefully clarified some with included documentation and unit tests.

[youknowriad]

Thanks Andrew, confirmed it's working properly.