Skip to content

Commit

Permalink
Allow flushSync to noop in life cycles but with a warning
Browse files Browse the repository at this point in the history
  • Loading branch information
sebmarkbage committed Apr 28, 2020
1 parent 4b02b66 commit 2b80075
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 29 deletions.
4 changes: 2 additions & 2 deletions packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ describe('ReactDOMFiberAsync', () => {
expect(ops).toEqual(['A', 'ABCD']);
});

it('flushSync throws if already performing work', () => {
it('flushSync logs an error if already performing work', () => {
class Component extends React.Component {
componentDidUpdate() {
ReactDOM.flushSync(() => {});
Expand All @@ -140,7 +140,7 @@ describe('ReactDOMFiberAsync', () => {
// Initial mount
ReactDOM.render(<Component />, container);
// Update
expect(() => ReactDOM.render(<Component />, container)).toThrow(
expect(() => ReactDOM.render(<Component />, container)).toErrorDev(
'flushSync was called from inside a lifecycle method',
);
});
Expand Down
17 changes: 10 additions & 7 deletions packages/react-reconciler/src/ReactFiberWorkLoop.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -1108,14 +1108,17 @@ export function unbatchedUpdates<A, R>(fn: (a: A) => R, a: A): R {
}

export function flushSync<A, R>(fn: A => R, a: A): R {
if ((executionContext & (RenderContext | CommitContext)) !== NoContext) {
invariant(
false,
'flushSync was called from inside a lifecycle method. It cannot be ' +
'called when React is already rendering.',
);
}
const prevExecutionContext = executionContext;
if ((prevExecutionContext & (RenderContext | CommitContext)) !== NoContext) {
if (__DEV__) {
console.error(
'flushSync was called from inside a lifecycle method. React cannot ' +
'flush when React is already rendering. Consider moving this call to ' +
'a scheduler task or micro task.',
);
}
return fn(a);
}
executionContext |= BatchedContext;
try {
return runWithPriority(ImmediatePriority, fn.bind(null, a));
Expand Down
17 changes: 10 additions & 7 deletions packages/react-reconciler/src/ReactFiberWorkLoop.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -1150,14 +1150,17 @@ export function unbatchedUpdates<A, R>(fn: (a: A) => R, a: A): R {
}

export function flushSync<A, R>(fn: A => R, a: A): R {
if ((executionContext & (RenderContext | CommitContext)) !== NoContext) {
invariant(
false,
'flushSync was called from inside a lifecycle method. It cannot be ' +
'called when React is already rendering.',
);
}
const prevExecutionContext = executionContext;
if ((prevExecutionContext & (RenderContext | CommitContext)) !== NoContext) {
if (__DEV__) {
console.error(
'flushSync was called from inside a lifecycle method. React cannot ' +
'flush when React is already rendering. Consider moving this call to ' +
'a scheduler task or micro task.',
);
}
return fn(a);
}
executionContext |= BatchedContext;
try {
return runWithPriority(ImmediatePriority, fn.bind(null, a));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1808,22 +1808,26 @@ describe('ReactHooksWithNoopRenderer', () => {
ReactNoop.flushSync(() => {
updateCount(props.count);
});
// This shouldn't flush synchronously.
expect(ReactNoop.getChildren()).not.toEqual([
span('Count: ' + props.count),
]);
}, [props.count]);
return <Text text={'Count: ' + count} />;
}
act(() => {
ReactNoop.render(<Counter count={0} />, () =>
Scheduler.unstable_yieldValue('Sync effect'),
);
expect(Scheduler).toFlushAndYieldThrough([
'Count: (empty)',
'Sync effect',
]);
expect(ReactNoop.getChildren()).toEqual([span('Count: (empty)')]);
expect(() => {
ReactNoop.flushPassiveEffects();
}).toThrow('flushSync was called from inside a lifecycle method');
});
expect(() =>
act(() => {
ReactNoop.render(<Counter count={0} />, () =>
Scheduler.unstable_yieldValue('Sync effect'),
);
expect(Scheduler).toFlushAndYieldThrough([
'Count: (empty)',
'Sync effect',
]);
expect(ReactNoop.getChildren()).toEqual([span('Count: (empty)')]);
}),
).toErrorDev('flushSync was called from inside a lifecycle method');
expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]);
});

it('unmounts previous effect', () => {
Expand Down

0 comments on commit 2b80075

Please sign in to comment.