ReactのComponent設計の修正

15 min read読了の目安(約14000字

概要

少し古いコードを手直しすることにしたのでまとめます。

患者

こちらになります。

GroupIndexPage.ts
(前略)
interface InnerPropsType {
  groupId: string,
  controller: Controller,
  group: Group,
  onLogout: () => void,
};

const GroupIndexPage = (props: InnerPropsType) => {
  const [message, setMessage] = useState<string>('');
  const [balance, setBalance] = useState<number|null>(null);
  const [records, setRecords] = useState<Record[]>([]);

  const loadTransactions = async() => {
    setBalance(await props.controller.getHolding(props.group.memberId));
    setRecords(await props.controller.getTransactions(props.group, props.group.memberId));
  }

  useEffect(()=>{
    loadTransactions();
  }, []);

  const onUpdateMemberName = async(value: string) => {
    try {
      await props.controller.updateMemberName(value);
    } catch(error) {
      setMessage(error.message);
    }
  };

  return (
    <GroupIndexTemplate
      message={message}
      groupId={props.groupId}
      groupName={props.group.groupName}
      tokenName={props.group.tokenName}
      logoUri={props.group.logoUri}
      balance={balance}
      sendTokenButton={
        <SendTokenButtonEx
          tokenName={props.group.tokenName}
          members={props.group.members}
          onSend={async(toMemberId: string, amount: number, comment: string)=>{
            await props.controller.send(props.group.memberId, toMemberId, amount, comment);
            loadTransactions();
          }}
        />
      }
      memberSettingsView={
        <MemberSettingsView
          memberId={props.group.memberId}
          memberName={props.group.members[props.group.memberId].displayName}
          onUpdateMemberName={onUpdateMemberName}
          onLogout={props.onLogout}
        />
      }
      historyPanel={
        <HistoryPanel records={records}/>
      }
    />
  );
};

interface LoadingPropsType {
  groupId: string,
  controller: Controller,
  onLogout: () => void,
};

const GroupIndexLoadingPage = (props: LoadingPropsType) => {
  const [group, setGroup] = useState<Group|null>(null);

  useEffect(()=>{
    const loadGroup = async() => {
      setGroup(await props.controller.getGroup());
    };

    loadGroup();
  }, [props.controller]);

  if (!group) {
    return (<p>loading...</p>);
  }

  return <GroupIndexPage group={group} {...props}/>
}

export default (props: PropsType) => {
  const groupId = props.match.params.id;
  const auth = GetCognitoAuth(null, null);

  if (!auth.isUserSignedIn()) {
    return (
      <div>
        <h2>サインインされていません</h2>
        <SignInButton callbackPath={props.location.pathname + props.location.search} />
      </div>
    );
  }

  if (!process.env.XXXX) {
    return <p>Application Error: process.env.XXXX is not set</p>;
  }

  const session = new Session(auth);
  const api = new RestApi(session, process.env.XXXX);

  return (
    <GroupIndexLoadingPage
      groupId={groupId}
      controller={new Controller(groupId, api)}
      onLogout={()=>session.close()}
    />
  );
};

診断(1回目)

ファイルデカすぎだしGroupIndexPage.tsのdefault exportがGroupIndexPageじゃないくて無名関数になってるのが一番キモいですけど、これはファイル分割すればいいので本質じゃないので後で。

一番なんとかしないといけないのは依存関係ですね。
ざっとTreeを書くとこんな感じ。

(無名関数)
  --> GroupIndexLoadingPage
    --> GroupIndexPage
      --> GroupIndexTemplate
      --> SendTokenButtonEx
      --> MemberSettingsView
      --> HistoryPanel

GroupIndexPageがTemplateとOrgamisms的Componentsに依存してるのはまぁいいのですが、それより上位階層がキモいです。なんでこんなにネスト深いんだ?

どうにもGroupIndexLoadingPageの役割がおかしいことが原因のようです。
データのロードとロード中画面生成に関して責務をおっているのにロード後画面生成は他コンポーネントに投げてます。

一度こいつをロードに集中させましょうか。
そうするとGroupIndexLoadingPageの関心はデータにあるべきでデータをもとに誰がどのように画面を生成するかは関心がないはずです。だから直接依存させるのもやめましょう。
遅延処理なのでDeplendencyInjectionはCallbackで解決します。

interface LoadGroupProps {
  controller: Controller,
  onLoading: () => JSX.Element,
  onLoaded:(group: Group) => JSX.Element,
};

const LoadGroup = ({controller, onLoading, onLoaded}: LoadGroupProps) => {
  const [group, setGroup] = useState<Group>();

  useEffect(()=>{
    controller.getGroup().then(setGroup).catch(()=>console.error("Fail to load"));
  }, [controller]);

  return group ? onLoaded(group) : onLoading();
}
 
interface PropsType {
  match: {params: {id: string}},
  location: History.Location<History.LocationState>,
};

export default (props: PropsType) => {
  const groupId = props.match.params.id;
  const auth = GetCognitoAuth(null, null);

  if (!auth.isUserSignedIn()) {
    return (
      <div>
        <h2>サインインされていません</h2>
        <SignInButton callbackPath={props.location.pathname + props.location.search} />
      </div>
    );
  }

  if (!process.env.XXXX) {
    return <p>Application Error: process.env.XXXX is not set</p>;
  }

  const session = new Session(auth);
  const controller = new Controller(groupId, new RestApi(session, process.env.XXXX));

  return (
    <LoadGroup
      controller={controller}
      onLoading={()=><p>loading...</p>}
      onLoaded={(group)=>(
        <GroupIndexPage
          group={group}
          groupId={groupId}
          controller={controller}
          onLogout={()=>session.close()}
        />
      )}
    />
  );
};

診断(2回目)

ついでに他のロード系の処理もおんなじ感じでパッケージ化しちゃいましょう。
環境変数読み込み処理もうざいので同様。


type LoadTransactionsProps = {
  controller: Controller,
  group: Group,
  render: (value: {balance: number|null, records: Record[], onUpdated: ()=>void}) => JSX.Element
};

const LoadTransactions = ({controller, group, render}: LoadTransactionsProps) => {
  const [balance, setBalance] = useState<number|null>(null);
  const [records, setRecords] = useState<Record[]>([]);

  const loadTransactions = async() => {
    setBalance(await controller.getHolding(group.memberId));
    setRecords(await controller.getTransactions(group, group.memberId));
  }

  useEffect(()=>{
    loadTransactions();
  }, []);

  return render({
    balance,
    records,
    onUpdated: loadTransactions,
  });
}

interface InnerPropsType {
  groupId: string,
  controller: Controller,
  group: Group,
  onLogout: () => void,
};

const GroupIndexPage = (props: InnerPropsType) => {
  const [message, setMessage] = useState<string>('');

  return (
    <LoadTransactions
      controller={props.controller}
      group={props.group}
      render={({balance, records, onUpdated})=>(
        <GroupIndexTemplate
          message={message}
          groupId={props.groupId}
          groupName={props.group.groupName}
          tokenName={props.group.tokenName}
          logoUri={props.group.logoUri}
          balance={balance}
          sendTokenButton={
            <SendTokenButtonEx
              tokenName={props.group.tokenName}
              members={props.group.members}
              onSend={async(toMemberId: string, amount: number, comment: string)=>{
                await props.controller.send(props.group.memberId, toMemberId, amount, comment);
                onUpdated();
              }}
            />
          }
          memberSettingsView={
            <MemberSettingsView
              memberId={props.group.memberId}
              memberName={props.group.members[props.group.memberId].displayName}
              onUpdateMemberName={async(value)=>{
                try {
                  await props.controller.updateMemberName(value);
                } catch(error) {
                  setMessage(error.message);
                }
              }}
              onLogout={props.onLogout}
            />
          }
          historyPanel={
            <HistoryPanel records={records}/>
          }
        />
      )}
    />
  );
};

interface LoadGroupProps {
  controller: Controller,
  onLoading: () => JSX.Element,
  onLoaded:(group: Group) => JSX.Element,
};

const LoadGroup = ({controller, onLoading, onLoaded}: LoadGroupProps) => {
  const [group, setGroup] = useState<Group>();

  useEffect(()=>{
    controller.getGroup().then(setGroup).catch(()=>console.error("Fail to load"));
  }, [controller]);

  return group ? onLoaded(group) : onLoading();
}
 
const UseSession = ({callbackPath, render}: {callbackPath: string, render: (params: {session: Session, onSignOut: ()=>void})=>JSX.Element}) => {
  const auth = GetCognitoAuth(null, null);

  if (!auth.isUserSignedIn()) {
    return (
      <div>
        <h2>サインインされていません</h2>
        <SignInButton callbackPath={callbackPath} />
      </div>
    );
  }
  const session = new Session(auth);

  return render({session, onSignOut: session.close});
};

type EnvironmentVariables = {
  apiUrl: string,
}

const LoadEnv = ({render}: {render: (env: EnvironmentVariables)=>JSX.Element}) => {
  if (!process.env.XXXX) {
    throw new Error("Application Error: process.env.XXXX is not set");
  }

  return render({
    apiUrl: process.env.XXXX,
  });
}

interface PropsType {
  match: {params: {id: string}},
  location: History.Location<History.LocationState>,
};

export default (props: PropsType) => {
  const groupId = props.match.params.id;

  return (
    <LoadEnv render={(env)=>(
      <UseSession
        callbackPath={props.location.pathname + props.location.search}
        render={({session, onSignOut})=> {
          const controller = new Controller(groupId, new RestApi(session, env.apiUrl));

          return (
            <LoadGroup
              controller={controller}
              onLoading={()=><p>loading...</p>}
              onLoaded={(group)=>(
                <GroupIndexPage
                  group={group}
                  groupId={groupId}
                  controller={controller}
                  onLogout={()=>onSignOut}
                />
              )}
            />
          )
        }}
      />
    )}/>
  );
};

診断(3回目)

ここまで来るとコンポーネント境界がおかしい事に気づく。
LoadEnv/UseSessionあたりはアプリケーション全体でコントロールすべきでpathに依存したgroupIdに依存する処理はGroupIndexPageが扱うべきだろう。
なのでそのへんをぐいっと入れ込む。


type LoadTransactionsProps = {
  controller: Controller,
  group: Group,
  render: (value: {balance: number|null, records: Record[], onUpdated: ()=>void}) => JSX.Element
};

const LoadTransactions = ({controller, group, render}: LoadTransactionsProps) => {
  const [balance, setBalance] = useState<number|null>(null);
  const [records, setRecords] = useState<Record[]>([]);

  const loadTransactions = async() => {
    setBalance(await controller.getHolding(group.memberId));
    setRecords(await controller.getTransactions(group, group.memberId));
  }

  useEffect(()=>{
    loadTransactions();
  }, []);

  return render({
    balance,
    records,
    onUpdated: loadTransactions,
  });
}

const GroupIndexPage = ({api, groupId, onSignOut}: {api: RestApi, groupId: string, onSignOut: ()=>void})=>{
  const [message, setMessage] = useState<string>('');

  const controller = new Controller(groupId, api);

  return (
    <LoadGroup
      controller={controller}
      onLoading={()=><p>loading...</p>}
      onLoaded={(group)=>(
        <LoadTransactions
          controller={controller}
          group={group}
          render={({balance, records, onUpdated})=>(
            <GroupIndexTemplate
              message={message}
              groupId={groupId}
              groupName={group.groupName}
              tokenName={group.tokenName}
              logoUri={group.logoUri}
              balance={balance}
              sendTokenButton={
                <SendTokenButtonEx
                  tokenName={group.tokenName}
                  members={group.members}
                  onSend={async(toMemberId: string, amount: number, comment: string)=>{
                    await controller.send(group.memberId, toMemberId, amount, comment);
                    onUpdated();
                  }}
                />
              }
              memberSettingsView={
                <MemberSettingsView
                  memberId={group.memberId}
                  memberName={group.members[group.memberId].displayName}
                  onUpdateMemberName={async(value)=>{
                    try {
                      await controller.updateMemberName(value);
                    } catch(error) {
                      setMessage(error.message);
                    }
                  }}
                  onLogout={onSignOut}
                />
              }
              historyPanel={
                <HistoryPanel records={records}/>
              }
            />
          )}
        />
      )}
    />
  );
}

interface LoadGroupProps {
  controller: Controller,
  onLoading: () => JSX.Element,
  onLoaded:(group: Group) => JSX.Element,
};

const LoadGroup = ({controller, onLoading, onLoaded}: LoadGroupProps) => {
  const [group, setGroup] = useState<Group>();

  useEffect(()=>{
    controller.getGroup().then(setGroup).catch(()=>console.error("Fail to load"));
  }, [controller]);

  return group ? onLoaded(group) : onLoading();
}
 
const UseSession = ({callbackPath, render}: {callbackPath: string, render: (params: {session: Session, onSignOut: ()=>void})=>JSX.Element}) => {
  const auth = GetCognitoAuth(null, null);

  if (!auth.isUserSignedIn()) {
    return (
      <div>
        <h2>サインインされていません</h2>
        <SignInButton callbackPath={callbackPath} />
      </div>
    );
  }
  const session = new Session(auth);

  return render({session, onSignOut: session.close});
};

type EnvironmentVariables = {
  apiUrl: string,
}

const LoadEnv = ({render}: {render: (env: EnvironmentVariables)=>JSX.Element}) => {
  if (!process.env.XXXX) {
    throw new Error("Application Error: process.env.XXXX is not set");
  }

  return render({
    apiUrl: process.env.XXXX,
  });
}

interface PropsType {
  match: {params: {id: string}},
  location: History.Location<History.LocationState>,
};

export default (props: PropsType) => {
  return (
    <LoadEnv render={(env)=>(
      <UseSession
        callbackPath={props.location.pathname + props.location.search}
        render={({session, onSignOut})=> (
          <GroupIndexPage
            api={new RestApi(session, env.apiUrl)}
            groupId={props.match.params.id}
            onSignOut={onSignOut}
          />
        )}
      />
    )}/>
  );
};

この先

一旦このファイルはここで終了。
このあとアプリ共通の処理は無名関数に集約されたので、無名関数は解体してReactRouterとかを扱ってるコンポーネントと統合するとスッキリするかと思います。