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

moving to aliased import #1264

Merged
merged 4 commits into from
Apr 17, 2019
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
20 changes: 18 additions & 2 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,23 @@ module.exports = {
// Allowing ++ on numbers
'no-plusplus': 'off',

// Disabling the use of !! to cast to boolean
// Nicer booleans
'no-restricted-syntax': [
// Disabling the use of !! to cast to boolean
'error',
{
selector:
'UnaryExpression[operator="!"] > UnaryExpression[operator="!"]',
message:
'!! to cast to boolean relies on a double negative. Use Boolean() instead',
},
// Avoiding accidental `new Boolean()` calls
// (also covered by `no-new-wrappers` but i am having fun)
{
selector: 'NewExpression[callee.name="Boolean"]',
message:
'Avoid using constructor: `new Boolean(value)` as it creates a Boolean object. Did you mean `Boolean(value)`?',
},
],

// Allowing Math.pow rather than forcing `**`
Expand All @@ -79,11 +87,19 @@ module.exports = {
'error',
{
paths: [
// Forcing use of useMemoOne
{
name: 'react',
importNames: ['useMemo', 'useCallback'],
message:
'useMemo and useCallback are subject to cache busting. Please use useMemoOne and useCallbackOne',
'`useMemo` and `useCallback` are subject to cache busting. Please use `useMemoOne`',
},
// Forcing use aliased imports from useMemoOne
{
name: 'use-memo-one',
importNames: ['useMemoOne', 'useCallbackOne'],
message:
'use-memo-one exports `useMemo` and `useCallback` which work nicer with `eslint-plugin-react-hooks`',
},
],
},
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
"react-redux": "^7.0.2",
"redux": "^4.0.1",
"tiny-invariant": "^1.0.4",
"use-memo-one": "^1.0.1"
"use-memo-one": "^1.1.0"
},
"devDependencies": {
"@atlaskit/css-reset": "^3.0.7",
Expand Down
82 changes: 51 additions & 31 deletions src/view/drag-drop-context/app.jsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
// @flow
import React, { useEffect, useRef, type Node } from 'react';
import invariant from 'tiny-invariant';
import { bindActionCreators } from 'redux';
import { Provider } from 'react-redux';
import { useMemoOne, useCallbackOne } from 'use-memo-one';
import { useMemo, useCallback } from 'use-memo-one';
Copy link
Contributor

Choose a reason for hiding this comment

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

Why? They may be confused with original react hooks.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It works nicer with eslint-plugin-react-hooks which matches on useMemo and useCallback

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is up to you though. You can still use import { useMemoOne } from 'use-memo-one'

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, okay. Didn't think about lint rule.

import createStore from '../../state/create-store';
import createDimensionMarshal from '../../state/dimension-marshal/dimension-marshal';
import canStartDrag from '../../state/can-start-drag';
Expand Down Expand Up @@ -32,6 +33,7 @@ import useAnnouncer from '../use-announcer';
import AppContext, { type AppContextValue } from '../context/app-context';
import useStartupValidation from './use-startup-validation';
import usePrevious from '../use-previous-ref';
import { warning } from '../../dev-warning';

type Props = {|
...Responders,
Expand All @@ -48,32 +50,36 @@ const createResponders = (props: Props): Responders => ({
onDragUpdate: props.onDragUpdate,
});

// flow does not support MutableRefObject
// type LazyStoreRef = MutableRefObject<?Store>;
type LazyStoreRef = {| current: ?Store |};

function getStore(lazyRef: LazyStoreRef): Store {
invariant(lazyRef.current, 'Could not find store from lazy ref');
return lazyRef.current;
}

export default function App(props: Props) {
const { uniqueId, setOnError } = props;
// flow does not support MutableRefObject
// let storeRef: MutableRefObject<Store>;
let storeRef;
const lazyStoreRef: LazyStoreRef = useRef<?Store>(null);

useStartupValidation();

// lazy collection of responders using a ref - update on ever render
const lastPropsRef = usePrevious<Props>(props);

const getResponders: () => Responders = useCallbackOne(() => {
const getResponders: () => Responders = useCallback(() => {
return createResponders(lastPropsRef.current);
}, []);
}, [lastPropsRef]);

const announce: Announce = useAnnouncer(uniqueId);
const styleMarshal: StyleMarshal = useStyleMarshal(uniqueId);

const lazyDispatch: Action => void = useCallbackOne(
(action: Action): void => {
storeRef.current.dispatch(action);
},
[],
);
const lazyDispatch: Action => void = useCallback((action: Action): void => {
getStore(lazyStoreRef).dispatch(action);
}, []);

const callbacks: DimensionMarshalCallbacks = useMemoOne(
const callbacks: DimensionMarshalCallbacks = useMemo(
() =>
bindActionCreators(
{
Expand All @@ -86,14 +92,14 @@ export default function App(props: Props) {
// $FlowFixMe - not sure why this is wrong
lazyDispatch,
),
[],
[lazyDispatch],
);
const dimensionMarshal: DimensionMarshal = useMemoOne<DimensionMarshal>(
const dimensionMarshal: DimensionMarshal = useMemo<DimensionMarshal>(
() => createDimensionMarshal(callbacks),
[],
[callbacks],
);

const autoScroller: AutoScroller = useMemoOne<AutoScroller>(
const autoScroller: AutoScroller = useMemo<AutoScroller>(
() =>
createAutoScroller({
scrollWindow,
Expand All @@ -106,10 +112,10 @@ export default function App(props: Props) {
lazyDispatch,
),
}),
[],
[dimensionMarshal.scrollDroppable, lazyDispatch],
);

const store: Store = useMemoOne<Store>(
const store: Store = useMemo<Store>(
() =>
createStore({
dimensionMarshal,
Expand All @@ -118,40 +124,54 @@ export default function App(props: Props) {
autoScroller,
getResponders,
}),
[],
[announce, autoScroller, dimensionMarshal, getResponders, styleMarshal],
);

storeRef = useRef<Store>(store);
// Checking for unexpected store changes
if (process.env.NODE_ENV !== 'production') {
if (lazyStoreRef.current && lazyStoreRef.current !== store) {
warning('unexpected store change');
}
}

// assigning lazy store ref
lazyStoreRef.current = store;

const tryResetStore = useCallbackOne(() => {
const state: State = storeRef.current.getState();
const tryResetStore = useCallback(() => {
const current: Store = getStore(lazyStoreRef);
const state: State = current.getState();
if (state.phase !== 'IDLE') {
store.dispatch(clean({ shouldFlush: true }));
current.dispatch(clean({ shouldFlush: true }));
}
}, []);

// doing this in render rather than a side effect so any errors on the
// initial mount are caught
setOnError(tryResetStore);

const getCanLift = useCallbackOne(
(id: DraggableId) => canStartDrag(storeRef.current.getState(), id),
const getCanLift = useCallback(
(id: DraggableId) => canStartDrag(getStore(lazyStoreRef).getState(), id),
[],
);

const getIsMovementAllowed = useCallbackOne(
() => isMovementAllowed(storeRef.current.getState()),
const getIsMovementAllowed = useCallback(
() => isMovementAllowed(getStore(lazyStoreRef).getState()),
[],
);

const appContext: AppContextValue = useMemoOne(
const appContext: AppContextValue = useMemo(
() => ({
marshal: dimensionMarshal,
style: styleMarshal.styleContext,
canLift: getCanLift,
isMovementAllowed: getIsMovementAllowed,
}),
[],
[
dimensionMarshal,
getCanLift,
getIsMovementAllowed,
styleMarshal.styleContext,
],
);

// Clean store when unmounting
Expand All @@ -161,7 +181,7 @@ export default function App(props: Props) {

return (
<AppContext.Provider value={appContext}>
<Provider context={StoreContext} store={storeRef.current}>
<Provider context={StoreContext} store={store}>
{props.children}
</Provider>
</AppContext.Provider>
Expand Down
4 changes: 2 additions & 2 deletions src/view/drag-drop-context/drag-drop-context.jsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// @flow
import React, { type Node } from 'react';
import { useMemoOne } from 'use-memo-one';
import { useMemo } from 'use-memo-one';
import type { Responders } from '../../types';
import ErrorBoundary from '../error-boundary';
import App from './app';
Expand All @@ -19,7 +19,7 @@ export function resetServerContext() {
}

export default function DragDropContext(props: Props) {
const uniqueId: number = useMemoOne(() => instanceCount++, []);
const uniqueId: number = useMemo(() => instanceCount++, []);

// We need the error boundary to be on the outside of App
// so that it can catch any errors caused by App
Expand Down
20 changes: 10 additions & 10 deletions src/view/draggable/draggable.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import { useRef } from 'react';
import { type Position } from 'css-box-model';
import invariant from 'tiny-invariant';
import { useMemoOne, useCallbackOne } from 'use-memo-one';
import { useMemo, useCallback } from 'use-memo-one';
import getStyle from './get-style';
import useDragHandle from '../use-drag-handle/use-drag-handle';
import type {
Expand All @@ -26,10 +26,10 @@ import useValidation from './use-validation';
export default function Draggable(props: Props) {
// reference to DOM node
const ref = useRef<?HTMLElement>(null);
const setRef = useCallbackOne((el: ?HTMLElement) => {
const setRef = useCallback((el: ?HTMLElement) => {
ref.current = el;
}, []);
const getRef = useCallbackOne((): ?HTMLElement => ref.current, []);
const getRef = useCallback((): ?HTMLElement => ref.current, []);

// context
const appContext: AppContextValue = useRequiredContext(AppContext);
Expand Down Expand Up @@ -63,7 +63,7 @@ export default function Draggable(props: Props) {
} = props;

// The dimension publisher: talks to the marshal
const forPublisher: DimensionPublisherArgs = useMemoOne(
const forPublisher: DimensionPublisherArgs = useMemo(
() => ({
draggableId,
index,
Expand All @@ -75,7 +75,7 @@ export default function Draggable(props: Props) {

// The Drag handle

const onLift = useCallbackOne(
const onLift = useCallback(
(options: { clientSelection: Position, movementMode: MovementMode }) => {
timings.start('LIFT');
const el: ?HTMLElement = ref.current;
Expand All @@ -93,12 +93,12 @@ export default function Draggable(props: Props) {
[draggableId, isDragDisabled, liftAction],
);

const getShouldRespectForcePress = useCallbackOne(
const getShouldRespectForcePress = useCallback(
() => shouldRespectForcePress,
[shouldRespectForcePress],
);

const callbacks: DragHandleCallbacks = useMemoOne(
const callbacks: DragHandleCallbacks = useMemo(
() => ({
onLift,
onMove: (clientSelection: Position) =>
Expand Down Expand Up @@ -130,7 +130,7 @@ export default function Draggable(props: Props) {
const isDropAnimating: boolean =
mapped.type === 'DRAGGING' && Boolean(mapped.dropping);

const dragHandleArgs: DragHandleArgs = useMemoOne(
const dragHandleArgs: DragHandleArgs = useMemo(
() => ({
draggableId,
isDragging,
Expand All @@ -155,7 +155,7 @@ export default function Draggable(props: Props) {

const dragHandleProps: ?DragHandleProps = useDragHandle(dragHandleArgs);

const onMoveEnd = useCallbackOne(
const onMoveEnd = useCallback(
(event: TransitionEvent) => {
if (mapped.type !== 'DRAGGING') {
return;
Expand All @@ -176,7 +176,7 @@ export default function Draggable(props: Props) {
[dropAnimationFinishedAction, mapped],
);

const provided: Provided = useMemoOne(() => {
const provided: Provided = useMemo(() => {
const style: DraggableStyle = getStyle(mapped);
const onTransitionEnd =
mapped.type === 'DRAGGING' && mapped.dropping ? onMoveEnd : null;
Expand Down
16 changes: 8 additions & 8 deletions src/view/droppable/droppable.jsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// @flow
import invariant from 'tiny-invariant';
import { useMemoOne, useCallbackOne } from 'use-memo-one';
import { useMemo, useCallback } from 'use-memo-one';
import React, { useRef, useContext, type Node } from 'react';
import type { Props, Provided } from './droppable-types';
import useDroppableDimensionPublisher from '../use-droppable-dimension-publisher';
Expand Down Expand Up @@ -40,22 +40,22 @@ export default function Droppable(props: Props) {
updateViewportMaxScroll,
} = props;

const getDroppableRef = useCallbackOne(
const getDroppableRef = useCallback(
(): ?HTMLElement => droppableRef.current,
[],
);
const getPlaceholderRef = useCallbackOne(
const getPlaceholderRef = useCallback(
(): ?HTMLElement => placeholderRef.current,
[],
);
const setDroppableRef = useCallbackOne((value: ?HTMLElement) => {
const setDroppableRef = useCallback((value: ?HTMLElement) => {
droppableRef.current = value;
}, []);
const setPlaceholderRef = useCallbackOne((value: ?HTMLElement) => {
const setPlaceholderRef = useCallback((value: ?HTMLElement) => {
placeholderRef.current = value;
}, []);

const onPlaceholderTransitionEnd = useCallbackOne(() => {
const onPlaceholderTransitionEnd = useCallback(() => {
// A placeholder change can impact the window's max scroll
if (isMovementAllowed()) {
updateViewportMaxScroll({ maxScroll: getMaxWindowScroll() });
Expand Down Expand Up @@ -96,7 +96,7 @@ export default function Droppable(props: Props) {
</AnimateInOut>
);

const provided: Provided = useMemoOne(
const provided: Provided = useMemo(
(): Provided => ({
innerRef: setDroppableRef,
placeholder,
Expand All @@ -107,7 +107,7 @@ export default function Droppable(props: Props) {
[placeholder, setDroppableRef, styleContext],
);

const droppableContext: ?DroppableContextValue = useMemoOne(
const droppableContext: ?DroppableContextValue = useMemo(
() => ({
droppableId,
type,
Expand Down
Loading