Skip to content

Commit

Permalink
Skip double invoking effects in Offscreen
Browse files Browse the repository at this point in the history
  • Loading branch information
sammy-SC committed Aug 5, 2022
1 parent 9fcaf88 commit b57ebbf
Show file tree
Hide file tree
Showing 2 changed files with 121 additions and 7 deletions.
28 changes: 21 additions & 7 deletions packages/react-reconciler/src/ReactFiberWorkLoop.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -2936,24 +2936,38 @@ function invokeEffectsInDev(

let current = firstChild;
let subtreeRoot = null;

const setNextCurrent = () => {
if (current === null) {
return;
}
if (current.sibling !== null) {
current = current.sibling;
} else {
current = subtreeRoot = current.return;
}
};

while (current !== null) {
const primarySubtreeFlag = current.subtreeFlags & fiberFlags;
if (
current !== subtreeRoot &&
current.child !== null &&
primarySubtreeFlag !== NoFlags
) {
current = current.child;
if (
current.child.tag === OffscreenComponent &&
current.child.pendingProps.mode === 'hidden'
) {
setNextCurrent();
} else {
current = current.child;
}
} else {
if ((current.flags & fiberFlags) !== NoFlags) {
invokeEffectFn(current);
}

if (current.sibling !== null) {
current = current.sibling;
} else {
current = subtreeRoot = current.return;
}
setNextCurrent();
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
let React;
let Offscreen;
let ReactDOMClient;
let act;

describe('ReactOffscreenStrictMode', () => {
let log;

beforeEach(() => {
jest.resetModules();
log = [];

React = require('react');
Offscreen = React.unstable_Offscreen;
ReactDOMClient = require('react-dom/client');
act = require('jest-react').act;

const ReactFeatureFlags = require('shared/ReactFeatureFlags');
ReactFeatureFlags.enableStrictEffects = __DEV__;
});

function Component({label}) {
React.useEffect(() => {
log.push(`${label}: useEffect mount`);
return () => log.push(`${label}: useEffect unmount`);
});

React.useLayoutEffect(() => {
log.push(`${label}: useLayoutEffect mount`);
return () => log.push(`${label}: useLayoutEffect unmount`);
});

log.push(`${label}: render`);

return null;
}

if (__DEV__) {
// @gate enableOffscreen
it('should trigger strict effects when offscreen is visible', () => {
const container = document.createElement('div');
const root = ReactDOMClient.createRoot(container, {
unstable_strictMode: true,
});

act(() => {
root.render(
<Offscreen mode="visible">
<Component label="A" />
</Offscreen>,
);
});

expect(log).toEqual([
'A: render',
'A: render',
'A: useLayoutEffect mount',
'A: useEffect mount',
'A: useLayoutEffect unmount',
'A: useEffect unmount',
'A: useLayoutEffect mount',
'A: useEffect mount',
]);
});

// @gate enableOffscreen
it('should not trigger strict effects when offscreen is hidden', () => {
const container = document.createElement('div');
const root = ReactDOMClient.createRoot(container, {
unstable_strictMode: true,
});

act(() => {
root.render(
<Offscreen mode="hidden">
<Component label="A" />
</Offscreen>,
);
});

expect(log).toEqual(['A: render', 'A: render']);
});
}

// @gate enableOffscreen
it('should default to no strinct effects', () => {

This comment has been minimized.

Copy link
@apostolos

apostolos Aug 9, 2022

typo "strinct" -> "strict"

const container = document.createElement('div');
const root = ReactDOMClient.createRoot(container);

act(() => {
root.render(
<Offscreen mode="hidden">
<Component label="A" />
</Offscreen>,
);
});

expect(log).toEqual(['A: render']);
});
});

0 comments on commit b57ebbf

Please sign in to comment.