1. はじめに
以下のロードマップ記事のChapter2として、UdemyのReact入門コースで動画を視聴した後、学んだ内容をもとに2-2. 課題1に挑戦してみました。
参考にしたロードマップ:
https://qiita.com/Sicut_study/items/6adaeb14abfe022e689c
なお、受講したUdemyコースの内容は前回投稿したReact入門記事と共通する点が多かったため、今回は割愛します。
useState・reduce・map を使って一通り動くものは完成しましたが、AIに添削してもらったところ「型の混在」と「Stateの安全な更新」という2つの改善ポイントを指摘されました。この記事では自分が最初に書いたコードと修正後のコードを比較しながら、気づいたことを整理します。
2. 2-2. 課題1に挑戦しました
Viteを使ってReact環境を用意するところから始め、仕様に沿って順番に機能を実装しました。
2.1 実装した機能一覧
以下の仕様をすべて実装しました。
- Viteを利用してReact環境を用意する
- Node.jsをインストールする
- Hello Worldが表示される
- タイトル「学習記録一覧」を見ることができる
- テストデータを一覧で表示する
- 学習内容の入力フォームをみることができる
- 学習時間の入力フォームをみることができる
- 学習時間の入力フォームは数字を入力できる
- 登録ボタンをみることができる
- 登録ボタンをクリックすると
recordsに記録を追加できる - 登録をしたらフォームが初期化される
- 全項目が入力されていないときにエラーが表示される
- 正しく入力されている場合登録ボタンを押すとエラーが消える
- 記録した勉強の時間を合計した値をみることができる
2.3 自分が書いたコード
はじめに実装したコード全体を示します。
import { useState } from "react";
import "./App.css";
export const Todo = () => {
// テキストと時間の状態管理
const [text, setText] = useState("");
// 初期値は数値の0
const [time, setTime] = useState(0);
// バリデーション表示フラグ
const [decision, setDecision] = useState(false);
// テストデータ
const [records, setRecord] = useState([
{ title: "勉強の記録1", time: 1 },
{ title: "勉強の記録2", time: 3 },
{ title: "勉強の記録3", time: 5 },
]);
const addRecord = () => {
// 未入力チェック
if (text === "" || time === 0) {
setDecision(true);
return;
} else {
setDecision(false);
}
// recordsを直接スプレッドして新しい配列を作成
const newRecord = [...records, { title: text, time: time }];
setRecord(newRecord);
// フォームをリセット
setText("");
setTime(0);
};
// 合計時間を計算(Number()で文字列を数値に変換している)
const calcTime = records.reduce((a, b) => {
return a + Number(b.time);
}, 0);
return (
<>
<div>
{/* タイトル */}
<h1>学習記録一覧</h1>
<label htmlFor="content">学習内容</label>
<input
value={text}
onChange={(e) => setText(e.target.value)}
id="content"
/>
<br />
<label htmlFor="time">学習時間</label>
{/* type="number"でも取り出した値は文字列になる */}
<input
type="number"
value={time}
onChange={(e) => setTime(e.target.value)}
id="time"
/>
時間
<p>入力されている学習内容:{text}</p>
<p>入力されている学習時間:{time}時間</p>
<ul>
{/* recordsをmapで一覧表示 */}
{records.map((record) => (
<li key={record.title}>
{record.title}:{record.time}時間
</li>
))}
</ul>
<button onClick={addRecord}>登録</button>
{decision && <p>入力されていない項目があります</p>}
<p>合計時間:{calcTime}/1000(h)</p>
</div>
</>
);
};
keyにはリスト内で一意となるIDを使うのが本来のベストプラクティスのようです。今回はテストデータにidがなかったため、代わりにtitleをkeyとして使っています。
動作はしていましたが、AIに添削してもらうと3つの改善ポイントが見つかりました。
3. AIに指摘されたポイント
自分では気づけなかった型とStateの扱い方について整理します。
3.1 e.target.valueは常に文字列になる
e.target.value で取得した値は、type="number" を指定していても必ず文字列として渡されるみたいです。
自分のコードでは setTime(e.target.value) と書いていたため、time の初期値 0(数値)と入力後の値(文字列)で型が混在した状態になっていました。
// 修正前:文字列がそのまま入ってしまう
onChange={(e) => setTime(e.target.value)}
// 修正後:Number()で数値に変換してから保存する
onChange={(e) => setTime(Number(e.target.value))}
3.2 型の混在でバリデーションが素通りする
time === 0 というバリデーションを書いていましたが、time に文字列の "0" が入っていると "0" === 0 が false になるため、バリデーションが効かなくなるようです。
===は型も含めて比較するため、"0"(文字列)と0(数値)は別物として扱われます。setTime(Number(e.target.value))に修正して数値として保存するようにすると、このバリデーションも正しく動くようになります。
3.3 prevを使ったStateの安全な更新
自分のコードでは以下のように書いていました。
// 修正前:recordsを直接参照している
const newRecord = [...records, { title: text, time: time }];
setRecord(newRecord);
連続でボタンを素早く押した場合に古い records を参照してしまう可能性があるとのことでした。
// 修正後:prevを使って最新のStateを安全に参照する
setRecord((prev) => [...prev, { title: text, time: time }]);
prev はReactが「今の最新の状態」を渡してくれる引数のようです。この書き方のほうが安全と理解しました。
prevを使う更新方法は「関数型更新」と呼ばれます。非同期処理が絡む場面や連続クリックが起きうる場面で特に効果を発揮するみたいです。
3.4 reduceのNumber()が不要になる
setTime の段階で数値に変換するようにしたため、reduce 内の Number() が不要になりました。
// 修正前:reduceの中でNumber()変換が必要だった
const calcTime = records.reduce((a, b) => {
return a + Number(b.time);
}, 0);
// 修正後:保存時に数値化済みなのでそのまま足せる
const calcTime = records.reduce((a, b) => {
return a + b.time;
}, 0);
4. 修正後のコード全体
3つの修正をすべて反映したコードです。
import { useState } from "react";
import "./App.css";
export const Todo = () => {
const [text, setText] = useState("");
// 初期値は数値の0
const [time, setTime] = useState(0);
const [decision, setDecision] = useState(false);
const [records, setRecord] = useState([
{ title: "勉強の記録1", time: 1 },
{ title: "勉強の記録2", time: 3 },
{ title: "勉強の記録3", time: 5 },
]);
const addRecord = () => {
// 未入力チェック(timeは数値なのでtime === 0が正しく機能する)
if (text === "" || time === 0) {
setDecision(true);
return;
} else {
setDecision(false);
}
// prevを使って最新のStateを安全に参照する
setRecord((prev) => [...prev, { title: text, time: time }]);
// フォームをリセット
setText("");
setTime(0);
};
// 保存時に数値化済みなのでNumber()が不要
const calcTime = records.reduce((a, b) => {
return a + b.time;
}, 0);
return (
<>
<div>
{/* タイトル */}
<h1>学習記録一覧</h1>
<label htmlFor="content">学習内容</label>
<input
value={text}
onChange={(e) => setText(e.target.value)}
id="content"
/>
<br />
<label htmlFor="time">学習時間</label>
{/* Number()で数値に変換してから保存する */}
<input
type="number"
value={time}
onChange={(e) => setTime(Number(e.target.value))}
id="time"
/>
時間
<p>入力されている学習内容:{text}</p>
<p>入力されている学習時間:{time}時間</p>
<ul>
{/* recordsをmapで一覧表示 */}
{records.map((record) => (
<li key={record.title}>
{record.title}:{record.time}時間
</li>
))}
</ul>
<button onClick={addRecord}>登録</button>
{decision && <p>入力されていない項目があります</p>}
<p>合計時間:{calcTime}/1000(h)</p>
</div>
</>
);
};
今回の気づき
「動いているから正しい」と思っていたコードに、型の混在という静かなバグが潜んでいたのが今回の一番の学びでした。type="number" のinputでも e.target.value は文字列になるという挙動は知らなければ気づけないポイントで、自分で書いたバリデーションが実は素通りしていたと知ったときは驚きました。また prev を使った関数型更新は「今は動いているけど安全ではない」という状態を防ぐための書き方のようで、正しい書き方を最初から習慣にしていきたいと思います。
ハマりやすいポイント
-
type="number"を指定していてもe.target.valueは文字列になるため、Number()で変換してからuseStateに渡す必要があります。 -
===は型も比較するため、"0" === 0はfalseになり、バリデーションが意図せず素通りしてしまいます。 -
setRecordに直接スプレッドした配列を渡す書き方は今は動いても、連続クリック時に古いStateを参照するリスクがあります。
