React Router unstable_middleware와 shouldRevalidate이 충돌하는 이슈 수정

이슈: Issue #14257
PR: PR #14286
릴리스: v 7.9.0

문제 발견

Issue #14257

React Router v7.8.2에서 unstable_middleware와 shouldRevalidate를 함께 사용할 때 버그가 보고되었습니다. 부모 라우트에서 shouldRevalidate: () => false를 설정했음에도 불구하고, 자식 라우트로 네비게이션할 때마다 부모 로더가 계속 재실행되는 문제였습니다.

재현 조건은 아래와 같았습니다.

SSR 모드 활성화 (ssr: true)
미들웨어 활성화 (unstable_middleware: true)
부모 라우트에 shouldRevalidate: () => false 설정

export const shouldRevalidate = () => false;

export const loader = async () => {
  console.log('parent-loader executed');
  return { data: 'parent data' };
};

또 이슈에 언급된 사실은 이 문제가 v7.8.1까지는 발생하지 않았다는 것이었습니다. v7.8.2에서 도입된 변경사항이 이 버그를 만들어낸 것이었죠.

원인 분석

버전 간 차이를 분석하기 위해 v7.8.1과 v7.8.2의 코드를 비교하기 시작했습니다. 특히 미들웨어 관련 코드와 재검증 로직에 집중했습니다.

packages/react-router/lib/server-runtime/single-fetch.ts 파일에서 핵심을 발견했습니다

// singleFetchAction 함수 내부
let result = await staticHandler.query(request, {
  skipRevalidation: true, // 재검증 스킵 옵션 설정
  generateMiddlewareResponse: build.future.v8_middleware
    ? async (query) => {
        try {
          let innerResult = await query(handlerRequest);
          // skipRevalidation이 전달되지 않음
          return handleQueryResult(innerResult);
        } catch (error) {
          return handleQueryError(error);
        }
      }
    : undefined,
});

skipRevalidation: true 옵션이 설정되어 있지만, generateMiddlewareResponse 콜백 내부에서 query()를 호출할 때 이 옵션이 전달되지 않고 있었습니다.

데이터 흐름을 추적해보니 제가 생각한 문제의 흐름은 아래와 같았습니다.

singleFetchAction
  -> skipRevalidation: true 설정
  -> generateMiddlewareResponse 호출
  -> 내부 query() 호출 시 skipRevalidation 누락
  -> shouldRevalidate 무시되고 로더 재실행

타입 정의를 확인해보니 더 근본적인 문제가 있었습니다. generateMiddlewareResponse 콜백 시그니처에 skipRevalidation 파라미터가 아예 정의되어 있지 않았습니다.

// 기존 타입 정의
generateMiddlewareResponse?: (
  query: (
    r: Request,
    args?: {
      filterMatchesToLoad?: (match: AgnosticDataRouteMatch) => boolean;
      // skipRevalidation이 없음
    },
  ) => Promise<StaticHandlerContext | Response>,
) => MaybePromise<Response>;

첫 번째 PR 시도

PR #14286

문제를 해결하기 위해 두 가지 수정이 필요했습니다.

1. 옵션 전달 수정

generateMiddlewareResponse: build.future.v8_middleware
  ? async (query) => {
      try {
        let innerResult = await query(handlerRequest, {
          filterMatchesToLoad: (m) => !loadRouteIds || loadRouteIds.has(m.route.id),
          skipRevalidation: true // 옵션 추가
        });
        return handleQueryResult(innerResult);
      } catch (error) {
        return handleQueryError(error);
      }
    }
  : undefined,

2. 타입 정의 업데이트

generateMiddlewareResponse?: (
  query: (
    r: Request,
    args?: {
      filterMatchesToLoad?: (match: AgnosticDataRouteMatch) => boolean;
      skipRevalidation?: boolean; // 타입 추가
    },
  ) => Promise<StaticHandlerContext | Response>,
) => MaybePromise<Response>;

3. 통합 테스트 작성

수정사항을 검증하기 위해 새로운 통합 테스트를 작성했습니다.

사실 저는 e2e 테스트에 대한 많은 경험이 없었기 때문에 기존 다른 e2e 테스트를 많이 참고하여 작성하였습니다.

부모 라우트는 shouldRevalidate: () => false 설정
자식 라우트 간 네비게이션 시 부모 로더 호출 횟수 확인
v8_middleware true/false 두 경우 모두 테스트

이 PR에서 가장 큰 시간이 걸린 부분이 이 테스트 작성 부분이였던 것 같습니다.

const files = {
  'react-router.config.ts': reactRouterConfig({
    ssr: true,
    v8_middleware: false,
  }),
  'app/root.tsx': js`
    import { Links, Meta, Outlet, Scripts, redirect } from "react-router";
    export function loader({ params }) {
      if (!params.child) {
        throw redirect("/parent/1");
      }
      return null;
    }
    export default function Root() {
      return (
        <html lang="en">
          <head>
            <Meta />
            <Links />
          </head>
          <body>
            <Outlet />
            <Scripts />
          </body>
        </html>
      );
    }
  `,

  'app/routes/parent.tsx': js`
    import { Outlet, useLoaderData } from "react-router";
    export const shouldRevalidate = () => {
      console.log("[PARENT] shouldRevalidate called - returning false");
      return false;
    };
    let loaderCallCount = 0;
    export function loader() {
      loaderCallCount++;
      console.log("[PARENT_LOADER] Called: " + loaderCallCount + " times");
      return {
        data: "parent data",
        callCount: loaderCallCount,
        timestamp: Date.now(),
      };
    }
    export default function Parent() {
      const parentData = useLoaderData();
      if (!parentData) throw new Error("no parent data");
      return (
        <div style={{ padding: "20px" }}>
          <h1>Parent</h1>
          <div id="parent-loader-count" data-count={parentData.callCount}>
            Parent Loader Count: {parentData.callCount}
          </div>
          <div id="parent-timestamp" data-timestamp={parentData.timestamp}>
            Parent Timestamp: {parentData.timestamp}
          </div>
          <Outlet />
        </div>
      );
    }
  `,

  'app/routes/parent.$child.tsx': js`
    import { href, Link, useParams, useLoaderData } from "react-router";
    export const shouldRevalidate = () => {
      console.log("[CHILD] shouldRevalidate called - returning true");
      return true;
    };
    let loaderCallCount = 0;
    export function loader({ params }) {
      loaderCallCount++;
      console.log("[CHILD_LOADER] Child " + params.child + " - Called: " + loaderCallCount + " times");
      return {
        data: "child data",
        callCount: loaderCallCount,
        childNumber: params.child,
      };
    }
    export default function Child() {
      const childData = useLoaderData();
      if (!childData) throw new Error("no child data");
      const params = useParams();
      const nextChild = Number(params.child) + 1;
      return (
        <div style={{ padding: "20px" }}>
          <h1>Child: {params.child}</h1>
          <div id="child-loader-count" data-count={childData.callCount}>
            Child Loader Count: {childData.callCount}
          </div>
          <Link
            id="next-child-link"
            style={{
              border: "1px solid #CCC",
              marginTop: "20px",
              display: "inline-block",
              padding: "4px 8px",
            }}
            to={href("/parent/:child", {
              child: nextChild.toString(),
            })}
          >
            Go to next child
          </Link>
        </div>
      );
    }
  `,
};

test.describe('shouldRevalidate with middleware', () => {
  test('v8_middleware false - parent loader called once', async ({ page }) => {
    let fixture = await createFixture({
      files,
    });
    let appFixture = await createAppFixture(fixture);
    let app = new PlaywrightFixture(appFixture, page);

    await app.goto('/parent/1');
    await app.clickLink('/parent/2');
    await app.clickLink('/parent/3');
    await app.clickLink('/parent/4');

    const initialParentCount = await page
      .locator('#parent-loader-count')
      .getAttribute('data-count');
    expect(initialParentCount).toBe('1');

    const secondParentCount = await page
      .locator('#parent-loader-count')
      .getAttribute('data-count');

    expect(secondParentCount).toBe('1');

    const thirdParentCount = await page
      .locator('#parent-loader-count')
      .getAttribute('data-count');

    expect(thirdParentCount).toBe('1');
  });

  test('v8_middleware true - server execution tracking', async ({ page }) => {
    const filesWithMiddleware = {
      ...files,
      'react-router.config.ts': reactRouterConfig({
        ssr: true,
        v8_middleware: true,
      }),
    };

    let fixture = await createFixture({
      files: filesWithMiddleware,
    });
    let appFixture = await createAppFixture(fixture);
    let app = new PlaywrightFixture(appFixture, page);

    const parentLoaderRegex = /\[PARENT_LOADER\] Called: (\d+) times/;
    let maxParentLoaderCount = 0;

    const originalLog = console.log;
    console.log = (...args) => {
      const message = args.join(' ');
      const match = message.match(parentLoaderRegex);
      if (match) {
        maxParentLoaderCount = Math.max(
          maxParentLoaderCount,
          parseInt(match[1]),
        );
      }
      originalLog(...args);
    };

    await app.goto('/parent/1');
    await app.clickLink('/parent/2');
    await app.clickLink('/parent/3');
    await app.clickLink('/parent/4');

    expect(maxParentLoaderCount).toBe(1);
  });
});

사소한 PR 이슈

PR을 준비하면서 한 가지 이슈가 있었습니다. React Router의 Contributing 가이드라인에 따르면,

"All new features, bug-fixes, or anything that touches react-router code should be branched off of and merged into the dev branch."

모든 코드 변경은 dev 브랜치를 대상으로 해야 했습니다. 그런데 dev 브랜치는 v8_middleware를 사용하고, main 브랜치는 unstable_middleware를 사용하고 있었습니다.

이슈는 unstable_middleware에 대해 보고되었지만, 가이드라인을 따르려면 v8_middleware에 대해 수정해야 했습니다. 다행히 두 미들웨어는 동일한 코드 경로를 공유하고 있어서, v8_middleware에 대한 수정이 unstable_middleware에도 적용될 것이었습니다.

따라서 PR 본문에 이러한 내용을 추가하여 상황를 전달했습니다.

## Note on Middleware Selection

Per the contributing guidelines, all code changes must target the dev branch.
The dev branch uses v8_middleware (whereas main uses unstable_middleware),
so I implemented and tested the fix against v8_middleware to comply with
the guidelines. The underlying bug reproduces identically on both
unstable_middleware and v8_middleware; this PR fixes the shared path by
correctly forwarding skipRevalidation through generateMiddlewareResponse.

메인테이너의 피드백

메인테이너가 리뷰를 진행했습니다. 그는 제 접근 방식을 인정하면서도 더 간단한 해결책을 제시했습니다,

"Thank you for the PR @joseph0926! We can fix this in a slightly simpler manner by falling back on the internal filterMatchesToLoad (which was what we missed originally in #14154)"

메인테이너는 내부의 filterMatchesToLoad를 활용하여 더 간결하게 문제를 해결했습니다. 또한 제가 작성한 테스트를 기존 미들웨어 테스트 스위트에 통합하여 코드를 더 깔끔하게 정리했습니다.

최종 코드

generateMiddlewareResponse: build.future.v8_middleware
  ? async (query) => {
      try {
        // 내부 filterMatchesToLoad를 폴백으로 사용
        let innerResult = await query(handlerRequest, {
          filterMatchesToLoad: opts?.filterMatchesToLoad ||
            ((m) => !loadRouteIds || loadRouteIds.has(m.route.id)),
          skipRevalidation: true
        });
        return handleQueryResult(innerResult);
      } catch (error) {
        return handleQueryError(error);
      }
    }
  : undefined,

최종 머지와 릴리스

모든 테스트가 통과하고, 메인테이너의 개선사항이 반영된 후 PR이 머지되었습니다. 이 수정은 v7.9.0 릴리스에 포함되었습니다.

마무리

해당 이슈 수정에 가장 큰 부분은 "어떤 버전까지는 정상이고 어디부터 문제 발생"과 "테스트 케이스 작성"이였습니다.

해당 이슈 수정뿐만 아니라 저는 모든 수정에 아래 규칙을 지키려 노력합니다.

내 수정이 절대 기존 기능에 영향을 주면 안된다
문제가 해결되어야한다.

이러한 가이드를 지키면서 수정하다보니 가장 크게 느낀것이 테스트의 중요성이였던거 같습니다.

문제 수정은 사실 요새 AI도 존재하고 정확히 문제 지점만 정확히 알면 수정이 가능하지만, 이 수정으로 어떤 영향이 발생할까?를 파악하는 과정은 약간 노가다이기도 하고 테스트가 부족하면 검증이 안되기때문에 테스트의 중요성을 알게된거같습니다.