Skip to content

Commit

Permalink
Fix hooks warnings identified by prerelease version of react-hooks ES…
Browse files Browse the repository at this point in the history
…Lint plugin
  • Loading branch information
Brian Vaughn committed Feb 19, 2019
1 parent a314150 commit bec0733
Show file tree
Hide file tree
Showing 8 changed files with 72 additions and 38 deletions.
2 changes: 1 addition & 1 deletion shells/dev/app/ToDoList/List.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export default function List(props: Props) {
setUID(uid + 1);
setNewItemText('');
}
}, [newItemText]);
}, [newItemText, items, uid]);

const handleKeyPress = useCallback(
event => {
Expand Down
33 changes: 23 additions & 10 deletions src/devtools/views/Element.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,22 +25,35 @@ export default function ElementView({ index, style }: Props) {

const element = getElementAtIndex(index);

if (element == null) {
console.warn(`<ElementView> Could not find element at index ${index}`);
return null;
}

const { depth, displayName, id, key, type } = ((element: any): Element);
const id = element === null ? null : element.id;

const handleDoubleClick = useCallback(() => selectOwner(id), [id]);
const handleDoubleClick = useCallback(() => {
if (id !== null) {
selectOwner(id);
}
}, [id, selectOwner]);

// TODO Add click and key handlers for toggling element open/close state.

const handleClick = useCallback(
({ metaKey }) => selectElementByID(metaKey ? null : id),
[id]
({ metaKey }) => {
if (id !== null) {
selectElementByID(metaKey ? null : id);
}
},
[id, selectElementByID]
);

// Handle elements that are removed from the tree while an async render is in progress.
if (element == null) {
console.warn(`<ElementView> Could not find element at index ${index}`);

// This return needs to happen after hooks, since hooks can't be conditional.
return null;
}

const { depth, displayName, key, type } = ((element: any): Element);

const isSelected = selectedElementID === id;
const showDollarR =
isSelected && (type === ElementTypeClass || type === ElementTypeFunction);
Expand All @@ -58,7 +71,7 @@ export default function ElementView({ index, style }: Props) {
}}
>
<span className={styles.Component}>
<DisplayName displayName={displayName} id={id} />
<DisplayName displayName={displayName} id={((id: any): number)} />
{key && (
<Fragment>
&nbsp;<span className={styles.AttributeName}>key</span>=
Expand Down
2 changes: 1 addition & 1 deletion src/devtools/views/InspectedElementTree.js
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ function EditableValue({
setEditableValue(target.value);
}
},
[dataType, setEditableValue]
[dataType, overrideValueFn, path]
);

const handleKeyPress = useCallback(
Expand Down
9 changes: 6 additions & 3 deletions src/devtools/views/SelectedElement.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export default function SelectedElement(_: Props) {
});
}
}
}, [bridge, selectedElementID, store]);
}, [bridge, element, selectedElementID, store]);

// TODO Make "view source" button work

Expand Down Expand Up @@ -176,7 +176,10 @@ function InspectedElementView({
function OwnerView({ displayName, id }: { displayName: string, id: number }) {
const { selectElementByID } = useContext(TreeContext);

const handleClick = useCallback(() => selectElementByID(id), [id]);
const handleClick = useCallback(() => selectElementByID(id), [
id,
selectElementByID,
]);

return (
<div
Expand Down Expand Up @@ -260,7 +263,7 @@ function useInspectedElement(id: number | null): InspectedElement | null {

bridge.removeListener('inspectedElement', onInspectedElement);
};
}, [id]);
}, [bridge, id, idRef, store]);

return inspectedElement;
}
11 changes: 9 additions & 2 deletions src/devtools/views/SettingsContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ function SettingsContextController({ browserTheme, children }: Props) {
default:
throw Error(`Unsupported theme value "${theme}"`);
}
}, [theme]);
}, [browserTheme, theme]);

const value = useMemo(
() => ({
Expand All @@ -89,7 +89,14 @@ function SettingsContextController({ browserTheme, children }: Props) {
? compactLineHeight
: comfortableLineHeight,
}),
[displayDensity, setDisplayDensity, theme, setTheme]
[
comfortableLineHeight,
compactLineHeight,
displayDensity,
setDisplayDensity,
setTheme,
theme,
]
);

return (
Expand Down
6 changes: 1 addition & 5 deletions src/devtools/views/Tree.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,7 @@ export default function Tree(props: Props) {
return () => {
window.removeEventListener('keydown', handleKeyDown);
};
}, [
selectedElementIndex,
selectNextElementInTree,
selectPreviousElementInTree,
]);
}, [selectNextElementInTree, selectPreviousElementInTree]);

// Let react-window know to re-render any time the underlying tree data changes.
// This includes the owner context, since it controls a filtered view of the tree.
Expand Down
16 changes: 14 additions & 2 deletions src/devtools/views/TreeContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -595,7 +595,19 @@ function TreeContextController({ children }: {| children: React$Node |}) {
resetOwnerStack,
selectOwner,
}),
[state]
[
getElementAtIndex,
goToNextSearchResult,
goToPreviousSearchResult,
resetOwnerStack,
selectElementAtIndex,
selectElementByID,
selectNextElementInTree,
selectOwner,
selectPreviousElementInTree,
setSearchText,
state,
]
);

// Listen for host element selections.
Expand Down Expand Up @@ -632,7 +644,7 @@ function TreeContextController({ children }: {| children: React$Node |}) {
store.addListener('mutated', handleStoreMutated);

return () => store.removeListener('mutated', handleStoreMutated);
}, [dispatch, store]);
}, [dispatch, initialRevision, store]);

return <TreeContext.Provider value={value}>{children}</TreeContext.Provider>;
}
Expand Down
31 changes: 17 additions & 14 deletions src/devtools/views/hooks.js
Original file line number Diff line number Diff line change
@@ -1,34 +1,37 @@
// @flow

import { useLayoutEffect, useState } from 'react';
import { useCallback, useLayoutEffect, useState } from 'react';

// Forked from https://usehooks.com/useLocalStorage/
export function useLocalStorage<T>(
key: string,
initialValue: T
): [T, (value: T | (() => T)) => void] {
const getValueFromLocalStorage = () => {
const getValueFromLocalStorage = useCallback(() => {
try {
const item = window.localStorage.getItem(key);
return item ? JSON.parse(item) : initialValue;
} catch (error) {
console.log(error);
return initialValue;
}
};
}, [initialValue, key]);

const [storedValue, setStoredValue] = useState(getValueFromLocalStorage);

const setValue = value => {
try {
const valueToStore =
value instanceof Function ? value(storedValue) : value;
setStoredValue(valueToStore);
window.localStorage.setItem(key, JSON.stringify(valueToStore));
} catch (error) {
console.log(error);
}
};
const setValue = useCallback(
value => {
try {
const valueToStore =
value instanceof Function ? value(storedValue) : value;
setStoredValue(valueToStore);
window.localStorage.setItem(key, JSON.stringify(valueToStore));
} catch (error) {
console.log(error);
}
},
[key, storedValue]
);

// Listen for changes to this local storage value made from other windows.
// This enables the e.g. "⚛️ Elements" tab to update in response to changes from "⚛️ Settings".
Expand All @@ -45,7 +48,7 @@ export function useLocalStorage<T>(
return () => {
window.removeEventListener('storage', onStorage);
};
}, [key, storedValue]);
}, [getValueFromLocalStorage, key, storedValue, setValue]);

return [storedValue, setValue];
}

0 comments on commit bec0733

Please sign in to comment.