Skip to content

Commit 4a06d3f

Browse files
authored
Fix cascading renders with signals (#4966) (#4969)
1 parent 87e912c commit 4a06d3f

File tree

2 files changed

+63
-2
lines changed

2 files changed

+63
-2
lines changed

hooks/test/browser/useState.test.jsx

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ describe('useState', () => {
1717
});
1818

1919
afterEach(() => {
20+
Component.prototype.shouldComponentUpdate = undefined;
2021
teardown(scratch);
2122
});
2223

@@ -476,4 +477,64 @@ describe('useState', () => {
476477
expect(renders).to.equal(2);
477478
});
478479
});
480+
481+
it('Works when we combine strict equality, signals bail and state settling', () => {
482+
// In signals we bail when we are using no signals/computeds/....
483+
Component.prototype.shouldComponentUpdate = function (
484+
nextProps,
485+
nextState
486+
) {
487+
return false;
488+
};
489+
let setA, setB;
490+
491+
const fooContext = createContext();
492+
const barContext = createContext();
493+
494+
function FooProvider({ children }) {
495+
const [a, _setA] = useState(0);
496+
setA = _setA;
497+
return <fooContext.Provider value={a}>{children}</fooContext.Provider>;
498+
}
499+
500+
function BarProvider({ children }) {
501+
const [b, _setB] = useState(0);
502+
setB = _setB;
503+
return <barContext.Provider value={b}>{children}</barContext.Provider>;
504+
}
505+
506+
function Child() {
507+
const a = useContext(fooContext);
508+
const b = useContext(barContext);
509+
return (
510+
<p>
511+
{a}-{b}
512+
</p>
513+
);
514+
}
515+
516+
function App() {
517+
return (
518+
<FooProvider>
519+
<BarProvider>
520+
<Child />
521+
</BarProvider>
522+
</FooProvider>
523+
);
524+
}
525+
526+
render(<App />, scratch);
527+
expect(scratch.innerHTML).to.equal('<p>0-0</p>');
528+
529+
act(() => {
530+
// We update A first so that we have a top-down render going on
531+
setA(1);
532+
// The update will bail at B, we don't want sCU to run because
533+
// else we risk the state's _nextValue being settled too early
534+
// and thus applying the update too early and bailing on the subsequent
535+
// render due to the values already being applied.
536+
setB(1);
537+
});
538+
expect(scratch.innerHTML).to.equal('<p>1-1</p>');
539+
});
479540
});

src/diff/index.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,14 +183,14 @@ export function diff(
183183
}
184184

185185
if (
186+
newVNode._original == oldVNode._original ||
186187
(!(c._bits & COMPONENT_FORCE) &&
187188
c.shouldComponentUpdate != NULL &&
188189
c.shouldComponentUpdate(
189190
newProps,
190191
c._nextState,
191192
componentContext
192-
) === false) ||
193-
newVNode._original == oldVNode._original
193+
) === false)
194194
) {
195195
// More info about this here: https://gist.github.com/JoviDeCroock/bec5f2ce93544d2e6070ef8e0036e4e8
196196
if (newVNode._original != oldVNode._original) {

0 commit comments

Comments
 (0)