Skip to content

Commit

Permalink
Remove isEqual argument
Browse files Browse the repository at this point in the history
The isEqual argument mostly meant as a strawman. It has some known
flaws: chiefly, if the isEqual function must always be in sync with the
selector function; if isEqual changes, then isEqual is not valid for
that render, because it'd be comparing the previous type with a
new type.

We have a few other ideas for how to solve the underlying problem, but
in the meantime, let's remove it since we know it's not what we'll end
up shipping.
  • Loading branch information
acdlite committed Feb 11, 2021
1 parent 730d3a0 commit a22f61c
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 188 deletions.
1 change: 0 additions & 1 deletion packages/react-debug-tools/src/ReactDebugHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,6 @@ function useContext<T>(
function useSelectedContext<C, S>(
Context: ReactContext<C>,
selector: C => S,
isEqual: ((S, S) => boolean) | void,
): S {
const context = Context._currentValue;
const selection = selector(context);
Expand Down
1 change: 0 additions & 1 deletion packages/react-dom/src/server/ReactPartialRendererHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,6 @@ function useContext<T>(
function useSelectedContext<C, S>(
Context: ReactContext<C>,
selector: C => S,
isEqual: ((S, S) => boolean) | void,
): S {
if (__DEV__) {
currentHookNameInDev = 'useSelectedContext';
Expand Down
73 changes: 15 additions & 58 deletions packages/react-reconciler/src/ReactFiberHooks.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,6 @@ function updateWorkInProgressHook(): Hook {
function mountSelectedContext<C, S>(
Context: ReactContext<C>,
selector: C => S,
isEqual: ((S, S) => boolean) | void,
): S {
if (!enableContextSelectors) {
return (undefined: any);
Expand All @@ -654,7 +653,6 @@ function mountSelectedContext<C, S>(
function updateSelectedContext<C, S>(
Context: ReactContext<C>,
selector: C => S,
isEqual: ((S, S) => boolean) | void,
): S {
if (!enableContextSelectors) {
return (undefined: any);
Expand All @@ -664,20 +662,7 @@ function updateSelectedContext<C, S>(
const context = readContextInsideHook(Context);
const newSelection = selector(context);
const oldSelection: S = hook.memoizedState;
if (isEqual !== undefined) {
if (__DEV__) {
if (typeof isEqual !== 'function') {
console.error(
'The optional third argument to useSelectedContext must be a ' +
'function. Instead got: %s',
isEqual,
);
}
}
if (isEqual(newSelection, oldSelection)) {
return oldSelection;
}
} else if (is(newSelection, oldSelection)) {
if (is(newSelection, oldSelection)) {
return oldSelection;
}
markWorkInProgressReceivedUpdate();
Expand Down Expand Up @@ -2267,17 +2252,13 @@ if (__DEV__) {
mountHookTypesDev();
return readContext(context, observedBits);
},
useSelectedContext<C, S>(
context: ReactContext<C>,
selector: C => S,
isEqual: ((S, S) => boolean) | void,
): S {
useSelectedContext<C, S>(context: ReactContext<C>, selector: C => S): S {
currentHookNameInDev = 'useSelectedContext';
mountHookTypesDev();
const prevDispatcher = ReactCurrentDispatcher.current;
ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnMountInDEV;
try {
return mountSelectedContext(context, selector, isEqual);
return mountSelectedContext(context, selector);
} finally {
ReactCurrentDispatcher.current = prevDispatcher;
}
Expand Down Expand Up @@ -2416,17 +2397,13 @@ if (__DEV__) {
updateHookTypesDev();
return readContext(context, observedBits);
},
useSelectedContext<C, S>(
context: ReactContext<C>,
selector: C => S,
isEqual: ((S, S) => boolean) | void,
): S {
useSelectedContext<C, S>(context: ReactContext<C>, selector: C => S): S {
currentHookNameInDev = 'useSelectedContext';
updateHookTypesDev();
const prevDispatcher = ReactCurrentDispatcher.current;
ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnMountInDEV;
try {
return mountSelectedContext(context, selector, isEqual);
return mountSelectedContext(context, selector);
} finally {
ReactCurrentDispatcher.current = prevDispatcher;
}
Expand Down Expand Up @@ -2561,17 +2538,13 @@ if (__DEV__) {
updateHookTypesDev();
return readContext(context, observedBits);
},
useSelectedContext<C, S>(
context: ReactContext<C>,
selector: C => S,
isEqual: ((S, S) => boolean) | void,
): S {
useSelectedContext<C, S>(context: ReactContext<C>, selector: C => S): S {
currentHookNameInDev = 'useSelectedContext';
updateHookTypesDev();
const prevDispatcher = ReactCurrentDispatcher.current;
ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV;
try {
return updateSelectedContext(context, selector, isEqual);
return updateSelectedContext(context, selector);
} finally {
ReactCurrentDispatcher.current = prevDispatcher;
}
Expand Down Expand Up @@ -2707,17 +2680,13 @@ if (__DEV__) {
updateHookTypesDev();
return readContext(context, observedBits);
},
useSelectedContext<C, S>(
context: ReactContext<C>,
selector: C => S,
isEqual: ((S, S) => boolean) | void,
): S {
useSelectedContext<C, S>(context: ReactContext<C>, selector: C => S): S {
currentHookNameInDev = 'useSelectedContext';
updateHookTypesDev();
const prevDispatcher = ReactCurrentDispatcher.current;
ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnRerenderInDEV;
try {
return updateSelectedContext(context, selector, isEqual);
return updateSelectedContext(context, selector);
} finally {
ReactCurrentDispatcher.current = prevDispatcher;
}
Expand Down Expand Up @@ -2855,18 +2824,14 @@ if (__DEV__) {
mountHookTypesDev();
return readContext(context, observedBits);
},
useSelectedContext<C, S>(
context: ReactContext<C>,
selector: C => S,
isEqual: ((S, S) => boolean) | void,
): S {
useSelectedContext<C, S>(context: ReactContext<C>, selector: C => S): S {
currentHookNameInDev = 'useSelectedContext';
warnInvalidHookAccess();
mountHookTypesDev();
const prevDispatcher = ReactCurrentDispatcher.current;
ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnMountInDEV;
try {
return mountSelectedContext(context, selector, isEqual);
return mountSelectedContext(context, selector);
} finally {
ReactCurrentDispatcher.current = prevDispatcher;
}
Expand Down Expand Up @@ -3016,18 +2981,14 @@ if (__DEV__) {
updateHookTypesDev();
return readContext(context, observedBits);
},
useSelectedContext<C, S>(
context: ReactContext<C>,
selector: C => S,
isEqual: ((S, S) => boolean) | void,
): S {
useSelectedContext<C, S>(context: ReactContext<C>, selector: C => S): S {
currentHookNameInDev = 'useSelectedContext';
warnInvalidHookAccess();
updateHookTypesDev();
const prevDispatcher = ReactCurrentDispatcher.current;
ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV;
try {
return updateSelectedContext(context, selector, isEqual);
return updateSelectedContext(context, selector);
} finally {
ReactCurrentDispatcher.current = prevDispatcher;
}
Expand Down Expand Up @@ -3178,18 +3139,14 @@ if (__DEV__) {
updateHookTypesDev();
return readContext(context, observedBits);
},
useSelectedContext<C, S>(
context: ReactContext<C>,
selector: C => S,
isEqual: ((S, S) => boolean) | void,
): S {
useSelectedContext<C, S>(context: ReactContext<C>, selector: C => S): S {
currentHookNameInDev = 'useSelectedContext';
warnInvalidHookAccess();
updateHookTypesDev();
const prevDispatcher = ReactCurrentDispatcher.current;
ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV;
try {
return updateSelectedContext(context, selector, isEqual);
return updateSelectedContext(context, selector);
} finally {
ReactCurrentDispatcher.current = prevDispatcher;
}
Expand Down
73 changes: 15 additions & 58 deletions packages/react-reconciler/src/ReactFiberHooks.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,6 @@ function updateWorkInProgressHook(): Hook {
function mountSelectedContext<C, S>(
Context: ReactContext<C>,
selector: C => S,
isEqual: ((S, S) => boolean) | void,
): S {
if (!enableContextSelectors) {
return (undefined: any);
Expand All @@ -654,7 +653,6 @@ function mountSelectedContext<C, S>(
function updateSelectedContext<C, S>(
Context: ReactContext<C>,
selector: C => S,
isEqual: ((S, S) => boolean) | void,
): S {
if (!enableContextSelectors) {
return (undefined: any);
Expand All @@ -664,20 +662,7 @@ function updateSelectedContext<C, S>(
const context = readContextInsideHook(Context);
const newSelection = selector(context);
const oldSelection: S = hook.memoizedState;
if (isEqual !== undefined) {
if (__DEV__) {
if (typeof isEqual !== 'function') {
console.error(
'The optional third argument to useSelectedContext must be a ' +
'function. Instead got: %s',
isEqual,
);
}
}
if (isEqual(newSelection, oldSelection)) {
return oldSelection;
}
} else if (is(newSelection, oldSelection)) {
if (is(newSelection, oldSelection)) {
return oldSelection;
}
markWorkInProgressReceivedUpdate();
Expand Down Expand Up @@ -2267,17 +2252,13 @@ if (__DEV__) {
mountHookTypesDev();
return readContext(context, observedBits);
},
useSelectedContext<C, S>(
context: ReactContext<C>,
selector: C => S,
isEqual: ((S, S) => boolean) | void,
): S {
useSelectedContext<C, S>(context: ReactContext<C>, selector: C => S): S {
currentHookNameInDev = 'useSelectedContext';
mountHookTypesDev();
const prevDispatcher = ReactCurrentDispatcher.current;
ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnMountInDEV;
try {
return mountSelectedContext(context, selector, isEqual);
return mountSelectedContext(context, selector);
} finally {
ReactCurrentDispatcher.current = prevDispatcher;
}
Expand Down Expand Up @@ -2416,17 +2397,13 @@ if (__DEV__) {
updateHookTypesDev();
return readContext(context, observedBits);
},
useSelectedContext<C, S>(
context: ReactContext<C>,
selector: C => S,
isEqual: ((S, S) => boolean) | void,
): S {
useSelectedContext<C, S>(context: ReactContext<C>, selector: C => S): S {
currentHookNameInDev = 'useSelectedContext';
updateHookTypesDev();
const prevDispatcher = ReactCurrentDispatcher.current;
ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnMountInDEV;
try {
return mountSelectedContext(context, selector, isEqual);
return mountSelectedContext(context, selector);
} finally {
ReactCurrentDispatcher.current = prevDispatcher;
}
Expand Down Expand Up @@ -2561,17 +2538,13 @@ if (__DEV__) {
updateHookTypesDev();
return readContext(context, observedBits);
},
useSelectedContext<C, S>(
context: ReactContext<C>,
selector: C => S,
isEqual: ((S, S) => boolean) | void,
): S {
useSelectedContext<C, S>(context: ReactContext<C>, selector: C => S): S {
currentHookNameInDev = 'useSelectedContext';
updateHookTypesDev();
const prevDispatcher = ReactCurrentDispatcher.current;
ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV;
try {
return updateSelectedContext(context, selector, isEqual);
return updateSelectedContext(context, selector);
} finally {
ReactCurrentDispatcher.current = prevDispatcher;
}
Expand Down Expand Up @@ -2707,17 +2680,13 @@ if (__DEV__) {
updateHookTypesDev();
return readContext(context, observedBits);
},
useSelectedContext<C, S>(
context: ReactContext<C>,
selector: C => S,
isEqual: ((S, S) => boolean) | void,
): S {
useSelectedContext<C, S>(context: ReactContext<C>, selector: C => S): S {
currentHookNameInDev = 'useSelectedContext';
updateHookTypesDev();
const prevDispatcher = ReactCurrentDispatcher.current;
ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnRerenderInDEV;
try {
return updateSelectedContext(context, selector, isEqual);
return updateSelectedContext(context, selector);
} finally {
ReactCurrentDispatcher.current = prevDispatcher;
}
Expand Down Expand Up @@ -2855,18 +2824,14 @@ if (__DEV__) {
mountHookTypesDev();
return readContext(context, observedBits);
},
useSelectedContext<C, S>(
context: ReactContext<C>,
selector: C => S,
isEqual: ((S, S) => boolean) | void,
): S {
useSelectedContext<C, S>(context: ReactContext<C>, selector: C => S): S {
currentHookNameInDev = 'useSelectedContext';
warnInvalidHookAccess();
mountHookTypesDev();
const prevDispatcher = ReactCurrentDispatcher.current;
ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnMountInDEV;
try {
return mountSelectedContext(context, selector, isEqual);
return mountSelectedContext(context, selector);
} finally {
ReactCurrentDispatcher.current = prevDispatcher;
}
Expand Down Expand Up @@ -3016,18 +2981,14 @@ if (__DEV__) {
updateHookTypesDev();
return readContext(context, observedBits);
},
useSelectedContext<C, S>(
context: ReactContext<C>,
selector: C => S,
isEqual: ((S, S) => boolean) | void,
): S {
useSelectedContext<C, S>(context: ReactContext<C>, selector: C => S): S {
currentHookNameInDev = 'useSelectedContext';
warnInvalidHookAccess();
updateHookTypesDev();
const prevDispatcher = ReactCurrentDispatcher.current;
ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV;
try {
return updateSelectedContext(context, selector, isEqual);
return updateSelectedContext(context, selector);
} finally {
ReactCurrentDispatcher.current = prevDispatcher;
}
Expand Down Expand Up @@ -3178,18 +3139,14 @@ if (__DEV__) {
updateHookTypesDev();
return readContext(context, observedBits);
},
useSelectedContext<C, S>(
context: ReactContext<C>,
selector: C => S,
isEqual: ((S, S) => boolean) | void,
): S {
useSelectedContext<C, S>(context: ReactContext<C>, selector: C => S): S {
currentHookNameInDev = 'useSelectedContext';
warnInvalidHookAccess();
updateHookTypesDev();
const prevDispatcher = ReactCurrentDispatcher.current;
ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV;
try {
return updateSelectedContext(context, selector, isEqual);
return updateSelectedContext(context, selector);
} finally {
ReactCurrentDispatcher.current = prevDispatcher;
}
Expand Down
6 changes: 1 addition & 5 deletions packages/react-reconciler/src/ReactInternalTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -299,11 +299,7 @@ export type Dispatcher = {|
context: ReactContext<T>,
observedBits: void | number | boolean,
): T,
useSelectedContext<C, S>(
context: ReactContext<C>,
selector: (C) => S,
isEqual: ((S, S) => boolean) | void,
): S,
useSelectedContext<C, S>(context: ReactContext<C>, selector: (C) => S): S,
useRef<T>(initialValue: T): {|current: T|},
useEffect(
create: () => (() => void) | void,
Expand Down
Loading

0 comments on commit a22f61c

Please sign in to comment.