Skip to content

Commit

Permalink
不適切なuseEffectの使用にコメントを追加
Browse files Browse the repository at this point in the history
  • Loading branch information
toririm committed Nov 5, 2024
1 parent d134c62 commit cd3de82
Show file tree
Hide file tree
Showing 16 changed files with 79 additions and 0 deletions.
4 changes: 4 additions & 0 deletions mobile/app/routes/welcome._index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ export default function Welcome() {
documentSub({ converter: orderConverter }),
);

/**
* 注文が完了した際に音を鳴らす
* OK
*/
useEffect(() => {
if (!order?.readyAt) {
return;
Expand Down
3 changes: 3 additions & 0 deletions pos/app/components/functional/useCurrentTime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ import { useEffect, useState } from "react";
export const useCurrentTime = (interval: number) => {
const [currentTime, setCurrentTime] = useState(new Date());

/**
* OK
*/
useEffect(() => {
const intervalId = setInterval(() => {
setCurrentTime(new Date());
Expand Down
3 changes: 3 additions & 0 deletions pos/app/components/functional/useFocusRef.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ import { useEffect, useRef } from "react";
const useFocusRef = <T extends HTMLElement>(focus: boolean) => {
const DOMRef = useRef<T>(null);

/**
* OK
*/
useEffect(() => {
if (focus) {
DOMRef.current?.focus();
Expand Down
3 changes: 3 additions & 0 deletions pos/app/components/functional/usePreventNumberKeyUpDown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ import { useEffect } from "react";
* 上下キーで数値を増減させないEffect
*/
const usePreventNumberKeyUpDown = () => {
/**
* OK
*/
useEffect(() => {
const handler = (event: KeyboardEvent) => {
if (event.key === "ArrowUp" || event.key === "ArrowDown") {
Expand Down
4 changes: 4 additions & 0 deletions pos/app/components/functional/useSyncCahiserOrder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ export const useSyncCahiserOrder = (
order: OrderEntity,
syncOrder: (order: OrderEntity) => void,
) => {
/**
* BAD: stateの更新にはuseEffectを使わない
* https://ja.react.dev/learn/you-might-not-need-an-effect#notifying-parent-components-about-state-changes
*/
useEffect(() => {
syncOrder(order);
}, [order, syncOrder]);
Expand Down
4 changes: 4 additions & 0 deletions pos/app/components/molecules/AttractiveInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ const AttractiveInput = ({ focus, onTextSet, ...props }: props) => {
[],
);

/**
* BAD: stateの更新にはuseEffectを使わない
* https://ja.react.dev/learn/you-might-not-need-an-effect#notifying-parent-components-about-state-changes
*/
useEffect(() => {
onTextSet(text);
}, [text, onTextSet]);
Expand Down
4 changes: 4 additions & 0 deletions pos/app/components/molecules/AttractiveTextArea.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ const AttractiveTextArea = ({ focus, onTextSet, ...props }: props) => {
[],
);

/**
* BAD: stateの更新にはuseEffectを使わない
* https://ja.react.dev/learn/you-might-not-need-an-effect#notifying-parent-components-about-state-changes
*/
useEffect(() => {
onTextSet(text);
}, [text, onTextSet]);
Expand Down
3 changes: 3 additions & 0 deletions pos/app/components/organisms/ConfirmDrawer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ type props = ComponentProps<typeof DrawerPrimitive.Root> & {
const ConfirmDrawer = ({ children, focus, onConfirm, ...props }: props) => {
const buttonRef = useRef<HTMLButtonElement>(null);

/**
* OK
*/
useEffect(() => {
console.log("use eefect");
const wait = async () => {
Expand Down
4 changes: 4 additions & 0 deletions pos/app/components/organisms/DiscountInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@ const DiscountInput = memo(
[discountOrder],
);

/**
* BAD: useEffect内でstateを更新している
* https://ja.react.dev/learn/you-might-not-need-an-effect#notifying-parent-components-about-state-changes
*/
useEffect(() => {
if (isComplete && discountOrder) {
onDiscountOrderFind(discountOrder);
Expand Down
4 changes: 4 additions & 0 deletions pos/app/components/organisms/ItemAssign.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ const ItemAssign = memo(
}, [assignee, idx, mutateItem]);

// アサイン入力欄を閉じるときに保存
/**
* BAD: useEffect内でstateを更新している
* https://ja.react.dev/learn/you-might-not-need-an-effect#notifying-parent-components-about-state-changes
*/
useEffect(() => {
if (!focus) {
saveAssignInput();
Expand Down
16 changes: 16 additions & 0 deletions pos/app/components/organisms/OrderItemEdit.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ const OrderItemEdit = memo(
}, [itemFocus, onRemoveItem]);

// ↑・↓ が押されたときに itemFocus を移動
/**
* OK
*/
useEffect(() => {
const handler = (event: KeyboardEvent) => {
switch (event.key) {
Expand All @@ -88,6 +91,9 @@ const OrderItemEdit = memo(
}, [focus, moveItemFocus]);

// Enter が押されたときに assign 入力欄を開く
/**
* OK
*/
useEffect(() => {
const handler = (event: KeyboardEvent) => {
if (event.key === "Enter") {
Expand All @@ -104,6 +110,9 @@ const OrderItemEdit = memo(

// キー操作でアイテムを追加
// Backspace でアイテムを削除
/**
* OK
*/
useEffect(() => {
const handler = (event: KeyboardEvent) => {
if (!focus) return;
Expand All @@ -120,6 +129,9 @@ const OrderItemEdit = memo(
}, [focus, onAddItem, removeItem, editable]);

// focus が外れたときに itemFocus をリセット
/**
* OK
*/
useEffect(() => {
if (!focus) {
setItemFocus(-1);
Expand All @@ -130,6 +142,10 @@ const OrderItemEdit = memo(
}, [focus]);

// itemFocus が range 外に出ないように調整
/**
* BAD: useEffect内でstateを更新している
* https://ja.react.dev/learn/you-might-not-need-an-effect#notifying-parent-components-about-state-changes
*/
useEffect(() => {
setItemFocus((prev) =>
Math.min(order.items.length - 1, Math.max(-1, prev)),
Expand Down
3 changes: 3 additions & 0 deletions pos/app/components/organisms/SubmitSection.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ export const SubmitSection = ({ submitOrder, order, focus }: props) => {
[order],
);

/**
* OK
*/
useEffect(() => {
if (focus) {
buttonRef.current?.focus();
Expand Down
7 changes: 7 additions & 0 deletions pos/app/components/pages/CashierV2.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ const CashierV2 = ({ items, orders, submitPayload, syncOrder }: props) => {

usePreventNumberKeyUpDown();

/**
* BAD: useEffect内でstateを更新している
* https://ja.react.dev/learn/you-might-not-need-an-effect#notifying-parent-components-about-state-changes
*/
useEffect(() => {
newOrderDispatch({ type: "updateOrderId", orderId: nextOrderId });
}, [nextOrderId, newOrderDispatch]);
Expand Down Expand Up @@ -97,6 +101,9 @@ const CashierV2 = ({ items, orders, submitPayload, syncOrder }: props) => {
};
}, [proceedStatus, previousStatus, resetAll]);

/**
* OK
*/
useEffect(() => {
const handler = (event: KeyboardEvent) => {
const key = event.key;
Expand Down
3 changes: 3 additions & 0 deletions pos/app/label/printer.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ export const useRawPrinter = () => {
const ePosDeviceRef = useRef();
const printerRef = useRef();

/**
* OK
*/
useEffect(() => {
if (status === "init") {
connect();
Expand Down
4 changes: 4 additions & 0 deletions pos/app/routes/_header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ export default function BaseHeader() {
const isOnline = useOnlineStatus();
const isOperational = useOrderStat();

/**
* BAD
* https://ja.react.dev/learn/you-might-not-need-an-effect#initializing-the-application
*/
useEffect(() => {
onAuthStateChanged(auth, (user) => {
if (user?.emailVerified) {
Expand Down
10 changes: 10 additions & 0 deletions pos/app/routes/cashier-mini.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,17 +39,27 @@ export default function CasherMini() {
return order?.orderId;
}, [order, logoShown, preOrder]);

/**
* BAD: useEffect内でstateを更新している
* https://ja.react.dev/learn/you-might-not-need-an-effect#notifying-parent-components-about-state-changes
*/
useEffect(() => {
setLogoShown(submittedOrderId != null || !isOperational);
}, [submittedOrderId, isOperational]);

/**
* OK
*/
useEffect(() => {
if (!logoShown) {
return;
}
videoRef.current?.play();
}, [logoShown]);

/**
* OK
*/
useEffect(() => {
if (submittedOrderId === null) {
return;
Expand Down

0 comments on commit cd3de82

Please sign in to comment.