Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up flushSync flow types #21887

Merged
merged 1 commit into from
Jul 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ describe('ReactDOMFiberAsync', () => {
it('flushSync logs an error if already performing work', () => {
class Component extends React.Component {
componentDidUpdate() {
ReactDOM.flushSync(() => {});
ReactDOM.flushSync();
}
render() {
return null;
Expand Down
5 changes: 1 addition & 4 deletions packages/react-noop-renderer/src/createReactNoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -913,10 +913,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
}
},

flushSync(fn: () => mixed) {
NoopRenderer.flushSync(fn);
},

flushSync: NoopRenderer.flushSync,
flushPassiveEffects: NoopRenderer.flushPassiveEffects,

// Logs the current state of the tree.
Expand Down
25 changes: 17 additions & 8 deletions packages/react-reconciler/src/ReactFiberWorkLoop.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -1093,10 +1093,13 @@ export function discreteUpdates<A, B, C, D, R>(
}
}

export function flushSyncWithoutWarningIfAlreadyRendering<A, R>(
fn: A => R,
a: A,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this second param is outdated. It's not used internally anywhere.

): R {
// Overload the definition to the two valid signatures.
// Warning, this opts-out of checking the function body.
declare function flushSyncWithoutWarningIfAlreadyRendering<R>(fn: () => R): R;
// eslint-disable-next-line no-redeclare
declare function flushSyncWithoutWarningIfAlreadyRendering(): void;
// eslint-disable-next-line no-redeclare
export function flushSyncWithoutWarningIfAlreadyRendering(fn) {
// In legacy mode, we flush pending passive effects at the beginning of the
// next event, not at the end of the previous one.
if (
Expand All @@ -1116,9 +1119,9 @@ export function flushSyncWithoutWarningIfAlreadyRendering<A, R>(
ReactCurrentBatchConfig.transition = 0;
setCurrentUpdatePriority(DiscreteEventPriority);
if (fn) {
return fn(a);
return fn();
} else {
return (undefined: $FlowFixMe);
return undefined;
}
} finally {
setCurrentUpdatePriority(previousPriority);
Expand All @@ -1133,7 +1136,13 @@ export function flushSyncWithoutWarningIfAlreadyRendering<A, R>(
}
}

export function flushSync<A, R>(fn: A => R, a: A): R {
// Overload the definition to the two valid signatures.
// Warning, this opts-out of checking the function body.
declare function flushSync<R>(fn: () => R): R;
// eslint-disable-next-line no-redeclare
declare function flushSync(): void;
// eslint-disable-next-line no-redeclare
export function flushSync(fn) {
if (__DEV__) {
if ((executionContext & (RenderContext | CommitContext)) !== NoContext) {
console.error(
Expand All @@ -1143,7 +1152,7 @@ export function flushSync<A, R>(fn: A => R, a: A): R {
);
}
}
return flushSyncWithoutWarningIfAlreadyRendering(fn, a);
return flushSyncWithoutWarningIfAlreadyRendering(fn);
}

export function flushControlled(fn: () => mixed): void {
Expand Down
25 changes: 17 additions & 8 deletions packages/react-reconciler/src/ReactFiberWorkLoop.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -1093,10 +1093,13 @@ export function discreteUpdates<A, B, C, D, R>(
}
}

export function flushSyncWithoutWarningIfAlreadyRendering<A, R>(
fn: A => R,
a: A,
): R {
// Overload the definition to the two valid signatures.
// Warning, this opts-out of checking the function body.
declare function flushSyncWithoutWarningIfAlreadyRendering<R>(fn: () => R): R;
// eslint-disable-next-line no-redeclare
declare function flushSyncWithoutWarningIfAlreadyRendering(): void;
// eslint-disable-next-line no-redeclare
export function flushSyncWithoutWarningIfAlreadyRendering(fn) {
// In legacy mode, we flush pending passive effects at the beginning of the
// next event, not at the end of the previous one.
if (
Expand All @@ -1116,9 +1119,9 @@ export function flushSyncWithoutWarningIfAlreadyRendering<A, R>(
ReactCurrentBatchConfig.transition = 0;
setCurrentUpdatePriority(DiscreteEventPriority);
if (fn) {
return fn(a);
return fn();
} else {
return (undefined: $FlowFixMe);
return undefined;
}
} finally {
setCurrentUpdatePriority(previousPriority);
Expand All @@ -1133,7 +1136,13 @@ export function flushSyncWithoutWarningIfAlreadyRendering<A, R>(
}
}

export function flushSync<A, R>(fn: A => R, a: A): R {
// Overload the definition to the two valid signatures.
// Warning, this opts-out of checking the function body.
declare function flushSync<R>(fn: () => R): R;
// eslint-disable-next-line no-redeclare
declare function flushSync(): void;
// eslint-disable-next-line no-redeclare
export function flushSync(fn) {
if (__DEV__) {
if ((executionContext & (RenderContext | CommitContext)) !== NoContext) {
console.error(
Expand All @@ -1143,7 +1152,7 @@ export function flushSync<A, R>(fn: A => R, a: A): R {
);
}
}
return flushSyncWithoutWarningIfAlreadyRendering(fn, a);
return flushSyncWithoutWarningIfAlreadyRendering(fn);
}

export function flushControlled(fn: () => mixed): void {
Expand Down
4 changes: 1 addition & 3 deletions packages/react-test-renderer/src/ReactTestRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -536,9 +536,7 @@ function create(element: React$Element<any>, options: TestRendererOptions) {
return getPublicRootInstance(root);
},

unstable_flushSync<T>(fn: () => T): T {
return flushSync(fn);
},
unstable_flushSync: flushSync,
};

Object.defineProperty(
Expand Down