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

不適切なuseEffectの使用にコメントを追加 #422

Merged
merged 1 commit into from
Feb 1, 2025
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
4 changes: 4 additions & 0 deletions services/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 services/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 services/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
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 services/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,
) => {
/**
* FIXME #412 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 services/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) => {
[],
);

/**
* FIXME #412 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 services/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) => {
[],
);

/**
* FIXME #412 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 services/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 services/pos/app/components/organisms/DiscountInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ const DiscountInput = memo(
[discountOrder],
);

/**
* FIXME #412 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 services/pos/app/components/organisms/ItemAssign.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ const ItemAssign = memo(
}, [assignee, idx, mutateItem]);

// アサイン入力欄を閉じるときに保存
/**
* FIXME #412 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 services/pos/app/components/organisms/OrderItemEdit.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ const OrderItemEdit = memo(
}, [itemFocus, onRemoveItem]);

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

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

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

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

// itemFocus が range 外に出ないように調整
/**
* FIXME #412 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 services/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 services/pos/app/components/pages/CashierV2.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ const CashierV2 = ({ items, orders, submitPayload, syncOrder }: props) => {

usePreventNumberKeyUpDown();

/**
* FIXME #412 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 @@ -95,6 +99,9 @@ const CashierV2 = ({ items, orders, submitPayload, syncOrder }: props) => {
};
}, [proceedStatus, previousStatus, resetAll]);

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

/**
* BAD
* https://ja.react.dev/learn/you-might-not-need-an-effect#initializing-the-application
*/
useEffect(() => {
if (status === "init") {
connect();
Expand Down
4 changes: 4 additions & 0 deletions services/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 services/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]);

/**
* FIXME #412 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
Loading