Skip to content

Commit

Permalink
Warn about incorrect use of useImperativeHandle()
Browse files Browse the repository at this point in the history
  • Loading branch information
gaearon committed Jan 21, 2019
1 parent b5a3df6 commit 133d256
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 0 deletions.
14 changes: 14 additions & 0 deletions packages/react-reconciler/src/ReactFiberHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -674,6 +674,12 @@ export function useImperativeHandle<T>(
): void {
if (__DEV__) {
currentHookNameInDev = 'useImperativeHandle';
warning(
typeof create === 'function',
'Expected useImperativeHandle() second argument to be a function ' +
'that creates a handle. Instead received: %s.',
create !== null ? typeof create : 'null',
);
}
// TODO: If deps are provided, should we skip comparing the ref itself?
const nextDeps =
Expand All @@ -690,6 +696,14 @@ export function useImperativeHandle<T>(
return () => refCallback(null);
} else if (ref !== null && ref !== undefined) {
const refObject = ref;
if (__DEV__) {
warning(
refObject.hasOwnProperty('current'),
'Expected useImperativeHandle() first argument to either be a ' +
'ref callback or React.createRef() object. Instead received: %s.',
'an object with keys {' + Object.keys(refObject).join(', ') + '}',
);
}
const inst = create();
refObject.current = inst;
return () => {
Expand Down
13 changes: 13 additions & 0 deletions packages/react-reconciler/src/ReactFiberScheduler.js
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,9 @@ function commitAllLifeCycles(
}
}
while (nextEffect !== null) {
if (__DEV__) {
setCurrentFiber(nextEffect);
}
const effectTag = nextEffect.effectTag;

if (effectTag & (Update | Callback)) {
Expand All @@ -513,6 +516,9 @@ function commitAllLifeCycles(

nextEffect = nextEffect.nextEffect;
}
if (__DEV__) {
resetCurrentFiber();
}
}

function commitPassiveEffects(root: FiberRoot, firstEffect: Fiber): void {
Expand All @@ -526,6 +532,10 @@ function commitPassiveEffects(root: FiberRoot, firstEffect: Fiber): void {

let effect = firstEffect;
do {
if (__DEV__) {
setCurrentFiber(effect);
}

if (effect.effectTag & Passive) {
let didError = false;
let error;
Expand All @@ -549,6 +559,9 @@ function commitPassiveEffects(root: FiberRoot, firstEffect: Fiber): void {
}
effect = effect.nextEffect;
} while (effect !== null);
if (__DEV__) {
resetCurrentFiber();
}

isRendering = previousIsRendering;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -569,6 +569,47 @@ describe('ReactHooks', () => {
]);
});

it('warns for bad useImperativeHandle first arg', () => {
const {useImperativeHandle} = React;
function App() {
useImperativeHandle({
focus() {},
});
return null;
}
let root;

expect(() => {
expect(() => {
root = ReactTestRenderer.create(<App />);
}).toThrow('create is not a function');
}).toWarnDev([
'Expected useImperativeHandle() first argument to either be a ' +
'ref callback or React.createRef() object. ' +
'Instead received: an object with keys {focus}.',
'Expected useImperativeHandle() second argument to be a function ' +
'that creates a handle. Instead received: undefined.',
]);
});

it('warns for bad useImperativeHandle second arg', () => {
const {useImperativeHandle} = React;
const App = React.forwardRef((props, ref) => {
useImperativeHandle(ref, {
focus() {},
});
return null;
});
let root;

expect(() => {
root = ReactTestRenderer.create(<App />);
}).toWarnDev([
'Expected useImperativeHandle() second argument to be a function ' +
'that creates a handle. Instead received: object.',
]);
});

// https://github.com/facebook/react/issues/14022
it('works with ReactDOMServer calls inside a component', () => {
const {useState} = React;
Expand Down

0 comments on commit 133d256

Please sign in to comment.