concrete-utopia / utopia

Design ❤️ Code
https://utopia.app
MIT License
3.75k stars 166 forks source link

useContextSelector memoization broken #1186

Open balazsbajorics opened 3 years ago

balazsbajorics commented 3 years ago

We used to have a custom fork of useContextSelector, I made that because I added an extra third parameter, an optional equality function.

When we deleted my code and went back to the mainline useContextSelector, we forgot to remove my own type definition, which masked the regression that the real useContextSelector uses Object.is as the equality function. This means that a lot of inspector hooks are now badly memoized!

(We did not realize this for months, because the inspector regression test, by accident was using an example code with lots of "spied" values, where the only value comes from the dom-walker's css computedStyle and not from the parsed props, so the regression tests are not testing if the parsed props are memoized or not)

Eni's removal of the layout prop meant that the regression test suddenly started working again, making us realize that there is a problem somewhere.

Rheeseyb commented 3 years ago

This has work done in #1226 , but needs further assessment

maltenuhn commented 2 years ago

@Rheeseyb how do we assess it? Or can this now be closed?

balazsbajorics commented 2 years ago

@maltenuhn

we need to make sure that our patch is working and is memoizing right, we can write tests to verify this.

we might as well update the dependency if we touch it anyways. (ideally write the tests first, to validate the API is stable)