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

profile取得をswrで実装してみた #5

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Conversation

exchange-wata
Copy link
Owner

@exchange-wata exchange-wata commented Feb 8, 2024

概要

  • リファクタ

実装内容

  • swrのinstall
  • api clientを叩くfetchをlibraryとして切り出し
  • profile取得のhooksを修正
    • 合わせてpage.tsxも修正
  • ロード中のcomponent(?)も追加

Comment on lines 19 to 24
if (isProfileLoading || !profile) return <Loading />;

return (
<div className='container mx-auto px-4 py-8'>
<div className='w-full max-w-xl mx-auto'>
{/* <Suspense fallback={<Loading />}> */}
Copy link
Owner Author

Choose a reason for hiding this comment

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

@レビュワーさん

  • suspenseを適用すると、初回ロード時のprofileがnullになる瞬間で落ちます
    • profile.imageUrlがprofile?.imageUrlにしないと怒られる感じです
  • profile?.~~を避けようとすると19行目みたいになるので、どっちがいいか判断つかなかったです.... ><

Choose a reason for hiding this comment

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

suspenseを使いたいときはSWRのoptionsに設定しないとダメです!
https://swr.vercel.app/ja/docs/suspense

ただし、useSWRに型引数を入れてしまうとsuspense: trueであってもdataの型にundefinedが追加されるみたいだから、

const { data } = useSWR(
  `/profile/find/${userId}`,
  fetcher as (url: string) => Promise<ProfileType>,
  {
    suspense: true,
  }
);

みたいにfetcher側にアサーションをつければいいと思う!

Copy link

@Nowe-melancholy Nowe-melancholy left a comment

Choose a reason for hiding this comment

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

基本的なSWRの使い方はいい感じ!
気になったところだけコメントした!

Comment on lines 5 to 15
export const useUserProfile = (userId: string) => {
const [profile, setProfile] = useState<ProfileType | null>(null);
const [error, setError] = useState<unknown | null>(null);
const { data, error, isLoading } = useSWR<ProfileType | null>(
`/profile/find/${userId}`,
fetcher
);

useEffect(() => {
const getUserProfile = async (userId: string) => {
if (!userId) return;
try {
const res = await apiClient.get(`/profile/find/${userId}`);

if (!res.data.profile)
throw new Error(`Profile not found, that userId is ${userId}`);

setProfile(res.data.profile);
} catch (error) {
console.log(error);
setError(error);
}
};

if (profile === null) getUserProfile(userId);
}, []);

return { profile, error };
return {
profile: data,
gettingProfileError: error,
isProfileLoading: isLoading,
};

Choose a reason for hiding this comment

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

カスタムフックでラップしないでuseSWRをコンポーネントで直接使っていいと思う!

Comment on lines 19 to 24
if (isProfileLoading || !profile) return <Loading />;

return (
<div className='container mx-auto px-4 py-8'>
<div className='w-full max-w-xl mx-auto'>
{/* <Suspense fallback={<Loading />}> */}

Choose a reason for hiding this comment

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

suspenseを使いたいときはSWRのoptionsに設定しないとダメです!
https://swr.vercel.app/ja/docs/suspense

ただし、useSWRに型引数を入れてしまうとsuspense: trueであってもdataの型にundefinedが追加されるみたいだから、

const { data } = useSWR(
  `/profile/find/${userId}`,
  fetcher as (url: string) => Promise<ProfileType>,
  {
    suspense: true,
  }
);

みたいにfetcher側にアサーションをつければいいと思う!

Copy link

@Nowe-melancholy Nowe-melancholy left a comment

Choose a reason for hiding this comment

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

めっちゃ色々コメントしたので、わからなかったらまた聞いてください!

const { data: profile } = useSWR(
`/profile/find/${params.userId}`,
fetcher<ProfileType>,
{ suspense: true }
Copy link

Choose a reason for hiding this comment

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

色々試してみたけどNext.js、SSR、suspense、SWRまわりはかなり複雑っぽくいので、整理したことをコメントしていきます。。。

まず、現状の実装では以下のようなエラーが出てるはず。
Uncaught Error: Fallback data is required when using suspense in SSR.

これに関してはどこが悪いのかピンポイントではわからなかった。。。
ただ、SWRのsuspenseがそもそも不安定なうえにSSR(app router?)との嚙み合いが悪いっぽいのが原因らしいです
vercel/swr#1906

Comment on lines +27 to +28
<ErrorBoundary fallback={<Error />}>
<Suspense fallback={<Loading />}>

Choose a reason for hiding this comment

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

ここの実装はたぶん想定通りに動いてないと思います。
ErrorBoundary、Suspenseはtry-catchのような役割をしていて、fetchしてるコンポーネントを囲わないといけないです。

じゃあなんでちゃんとローディングだったりエラー画面だったりが表示されているのかというと、これはNext.jsのapp routerの機能が動いてるからです。
つまり、Next.jsは内部的にpage.tsxを勝手にError BoundaryとSuspenseで囲ってて、page.tsxと同じ階層にあるerror.tsxとloading.tsxをfallbackに指定してるんですね。

https://nextjs.org/docs/app/building-your-application/routing/loading-ui-and-streaming
https://nextjs.org/docs/app/building-your-application/routing/error-handling

Choose a reason for hiding this comment

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

そうすると、「結局どうやって実装すればいいの?」ということになるけど、これは2通りの解決方法があると思います。

  • Next.jsの機能(SSR、app router)を使うのをやめてCSR + SWRに振り切る
  • Next.jsのapp routerに寄せる

どっちのコード例も書いていきます!

Choose a reason for hiding this comment

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

まず、「Next.jsの機能(SSR、app router)を使うのをやめてCSR + SWRに振り切る」から説明します。

修正点としては以下2つです。

  • Error Boundary, Suspenseを正しく囲む
  • SSRをやめる

「Error Boundary, Suspenseを正しく囲む」については、以下のような感じにするといいです!

'use client';
...

const UserProfile = ({
  params: { userId },
}: {
  params: { userId: string };
}) => {
  return (
    <div className='container mx-auto px-4 py-8'>
      <div className='w-full max-w-xl mx-auto'>
        <ErrorBoundary fallback={<Error />}>
          <Suspense fallback={<Loading />}>
            <Inner userId={userId} />
          </Suspense>
        </ErrorBoundary>
      </div>
    </div>
  );
};

const Inner = ({ userId }: { userId: string }) => {
  const { data: profile } = useSWR(
    `/profile/find/${userId}`,
    fetcher<ProfileType>,
    { suspense: true }
  );

  const { data: posts } = useSWR(`/posts/get/${userId}`, fetcher<PostType[]>, {
    suspense: true,
  });

  return (
    <>
      <div className='bg-white shadow-md rounded-lg p-6 mb-4'>
        <div className='flex items-center'>
          <img
            className='w-20 h-20 rounded-full mr-4'
            alt='User Avatar'
            src={profile.imageUrl}
          />
          <div>
            <h2 className='text-2xl font-semibold mb-1'>{profile.user.name}</h2>
            <p className='text-gray-600'>{profile.bio}</p>
          </div>
        </div>
      </div>
      {posts?.map((post: PostType) => (
        <div className='bg-white shadow-md rounded p-4 mb-4' key={post.id}>
          <div className='mb-4'>
            <div className='flex items-center mb-2'>
              <img
                className='w-10 h-10 rounded-full mr-2'
                alt='User Avatar'
                src={profile?.imageUrl}
              />
              <div>
                <h2 className='font-semibold text-md'>{post.author.name}</h2>
                <p className='text-gray-500 text-sm'>{post.createdAt}</p>
              </div>
            </div>
            <p className='text-gray-700'>{post.content}</p>
          </div>
        </div>
      ))}
    </>
  );
};

つまり、画面表示部分をInner Componentとして切り出して、そのInner ComponentをError Boundary, Suspenseで囲む、といった実装です。

「SSRをやめる」についてはexportを変更するだけです

export default dynamic(() => Promise.resolve(UserProfile), {
  ssr: false,
});

Choose a reason for hiding this comment

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

次に「Next.jsのapp routerに寄せる」について説明します!
つまり、このコンポーネントをServer Componentにしてしまうということですね。

この場合、SWRをやめて次のようにする感じです!
また、先頭のuse clientもErrorBoundaryもSuspenseも消しちゃいましょう!

const UserProfile = async ({ params }: { params: { userId: string } }) => {
  const profile = await fetcher<ProfileType>(`/profile/find/${params.userId}`);
  const posts = await fetcher<PostType[]>(`/posts/get/${params.userId}`);

  return (
    <div className='container mx-auto px-4 py-8'>
      <div className='w-full max-w-xl mx-auto'>
      ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants