Skip to content

Commit 7b74b40

Browse files
authored
Fix cascading renders with signals (#4966)
1 parent 3ab5c6f commit 7b74b40

2 files changed

Lines changed: 63 additions & 2 deletions

File tree

hooks/test/browser/useState.test.js

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

2121
afterEach(() => {
22+
Component.prototype.shouldComponentUpdate = undefined;
2223
teardown(scratch);
2324
});
2425

@@ -414,4 +415,64 @@ describe('useState', () => {
414415
expect(renders).to.equal(2);
415416
});
416417
});
418+
419+
it('Works when we combine strict equality, signals bail and state settling', () => {
420+
// In signals we bail when we are using no signals/computeds/....
421+
Component.prototype.shouldComponentUpdate = function (
422+
nextProps,
423+
nextState
424+
) {
425+
return false;
426+
};
427+
let setA, setB;
428+
429+
const fooContext = createContext();
430+
const barContext = createContext();
431+
432+
function FooProvider({ children }) {
433+
const [a, _setA] = useState(0);
434+
setA = _setA;
435+
return <fooContext.Provider value={a}>{children}</fooContext.Provider>;
436+
}
437+
438+
function BarProvider({ children }) {
439+
const [b, _setB] = useState(0);
440+
setB = _setB;
441+
return <barContext.Provider value={b}>{children}</barContext.Provider>;
442+
}
443+
444+
function Child() {
445+
const a = useContext(fooContext);
446+
const b = useContext(barContext);
447+
return (
448+
<p>
449+
{a}-{b}
450+
</p>
451+
);
452+
}
453+
454+
function App() {
455+
return (
456+
<FooProvider>
457+
<BarProvider>
458+
<Child />
459+
</BarProvider>
460+
</FooProvider>
461+
);
462+
}
463+
464+
render(<App />, scratch);
465+
expect(scratch.innerHTML).to.equal('<p>0-0</p>');
466+
467+
act(() => {
468+
// We update A first so that we have a top-down render going on
469+
setA(1);
470+
// The update will bail at B, we don't want sCU to run because
471+
// else we risk the state's _nextValue being settled too early
472+
// and thus applying the update too early and bailing on the subsequent
473+
// render due to the values already being applied.
474+
setB(1);
475+
});
476+
expect(scratch.innerHTML).to.equal('<p>1-1</p>');
477+
});
417478
});

src/diff/index.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,14 +165,14 @@ export function diff(
165165
}
166166

167167
if (
168+
newVNode._original == oldVNode._original ||
168169
(!c._force &&
169170
c.shouldComponentUpdate != NULL &&
170171
c.shouldComponentUpdate(
171172
newProps,
172173
c._nextState,
173174
componentContext
174-
) === false) ||
175-
newVNode._original == oldVNode._original
175+
) === false)
176176
) {
177177
// More info about this here: https://gist.github.com/JoviDeCroock/bec5f2ce93544d2e6070ef8e0036e4e8
178178
if (newVNode._original != oldVNode._original) {

0 commit comments

Comments
 (0)