ronantakizawa/codereview-bench huggingface dataset:
Repository: remix-run/react-router
File: packages/react-router/lib/dom/ssr/routes-test-stub.tsx (TypeScript)

Code before:
  hydrationData?: HydrationState;

  /**
   * Future flags mimicking the settings in react-router.config.ts
   */
  future?: Partial<FutureConfig>;
}

/**
 * @category Utils
 */
export function createRoutesStub(
  routes: StubRouteObject[],
  context: AppLoadContext = {}
) {
  return function RoutesTestStub({
    initialEntries,
    initialIndex,
    hydrationData,
    future,
  }: RoutesTestStubProps) {
    let routerRef = React.useRef<ReturnType<typeof createMemoryRouter>>();
    let reactRouterContextRef = React.useRef<FrameworkContextObject>();

    if (routerRef.current == null) {
      reactRouterContextRef.current = {
        future: {},
        manifest: {
          routes: {},
          entry: { imports: [], module: "" },
          url: "",
          version: "",
        },
        routeModules: {},
        isSpaMode: false,
      };

      // Update the routes to include context in the loader/action and populate
      // the manifest and routeModules during the walk
      let patched = processRoutes(
        // @ts-expect-error loader/action context types don't match :/
        convertRoutesToDataRoutes(routes, (r) => r),
        context,
        reactRouterContextRef.current.manifest,
        reactRouterContextRef.current.routeModules
      );
      routerRef.current = createMemoryRouter(patched, {
        initialEntries,
        initialIndex,
        hydrationData,
      });

Review comment (suggestion):
```suggestion
      frameworkContextRef.current = {
```

Code after:
   *  e.g. initialEntries={["/home", "/about", "/contact"]}
   */
  initialEntries?: InitialEntry[];

  /**
   * The initial index in the history stack to render. This allows you to start a test at a specific entry.
   * It defaults to the last entry in initialEntries.
   * e.g.
   *   initialEntries: ["/", "/events/123"]
   *   initialIndex: 1 // start at "/events/123"
   */
  initialIndex?: number;

  /**
   *  Used to set the route's initial loader and action data.
   *  e.g. hydrationData={{
   *   loaderData: { "/contact": { locale: "en-US" } },
   *   actionData: { "/login": { errors: { email: "invalid email" } }}
   *  }}
   */
  hydrationData?: HydrationState;

  /**
   * Future flags mimicking the settings in react-router.config.ts
   */
  future?: Partial<FutureConfig>;
}

/**
 * @category Utils
 */
export function createRoutesStub(
  routes: StubRouteObject[],
  unstable_getContext?: () => unstable_InitialContext
) {
  return function RoutesTestStub({
    initialEntries,
    initialIndex,
    hydrationData,
    future,
  }: RoutesTestStubProps) {
    let routerRef = React.useRef<ReturnType<typeof createMemoryRouter>>();
    let frameworkContextRef = React.useRef<FrameworkContextObject>();

    if (routerRef.current == null) {
      frameworkContextRef.current = {
        future: {
          unstable_subResourceIntegrity:
            future?.unstable_subResourceIntegrity === true,
          unstable_middleware: future?.unstable_middleware === true,
        },
Repository: remix-run/react-router
File: packages/react-router-dev/vite/cloudflare-dev-proxy.ts (TypeScript)

Code before:
import { createRequestHandler } from "react-router";
import { type AppLoadContext, type ServerBuild } from "react-router";
import { type Plugin } from "vite";
import { type GetPlatformProxyOptions, type PlatformProxy } from "wrangler";

import { fromNodeRequest, toNodeRequest } from "./node-adapter";
import { preloadViteEsm, importViteEsmSync } from "./import-vite-esm-sync";

let serverBuildId = "virtual:react-router/server-build";

type CfProperties = Record<string, unknown>;

type LoadContext<Env, Cf extends CfProperties> = {
  cloudflare: Omit<PlatformProxy<Env, Cf>, "dispose">;
};

type GetLoadContext<Env, Cf extends CfProperties> = (args: {
  request: Request;
  context: LoadContext<Env, Cf>;
}) => AppLoadContext | Promise<AppLoadContext>;

function importWrangler() {
  try {
    return import("wrangler");
  } catch (_) {
    throw Error("Could not import `wrangler`. Do you have it installed?");
  }
}

const PLUGIN_NAME = "react-router-cloudflare-vite-dev-proxy";

/**

Review comment (suggestion):
This has changed in the `dev` branch. Can you update and replace it with this:

```suggestion
import { preloadVite, getVite } from "./vite";
```

Code after:
import { createRequestHandler } from "react-router";
import { type AppLoadContext, type ServerBuild } from "react-router";
import { type Plugin } from "vite";
import { type GetPlatformProxyOptions, type PlatformProxy } from "wrangler";

import { fromNodeRequest, toNodeRequest } from "./node-adapter";
import { preloadVite, getVite } from "./vite";

let serverBuildId = "virtual:react-router/server-build";

type CfProperties = Record<string, unknown>;

type LoadContext<Env, Cf extends CfProperties> = {
  cloudflare: Omit<PlatformProxy<Env, Cf>, "dispose">;
};

type GetLoadContext<Env, Cf extends CfProperties> = (args: {
  request: Request;
  context: LoadContext<Env, Cf>;
}) => AppLoadContext | Promise<AppLoadContext>;

function importWrangler() {
  try {
    return import("wrangler");
  } catch (_) {
    throw Error("Could not import `wrangler`. Do you have it installed?");
  }
}

const PLUGIN_NAME = "react-router-cloudflare-vite-dev-proxy";

/**
Repository: remix-run/react-router
File: packages/react-router-dev/typegen/index.ts (TypeScript)

Code before:

async function createContext({
  rootDirectory,
  watch,
}: {
  rootDirectory: string;
  watch: boolean;
}): Promise<Context> {
  const configLoader = await createConfigLoader({ rootDirectory, watch });
  const configResult = await configLoader.getConfig();

  if (!configResult.ok) {
    throw new Error(configResult.error);
  }

  const config = configResult.value;

  return {
    configLoader,
    rootDirectory,
    config,
  };
}

function isRouteInAppDirectory(ctx: Context, route: RouteManifestEntry) {
  const absoluteRoutePath = Path.resolve(ctx.config.appDirectory, route.file);
  return absoluteRoutePath.startsWith(ctx.config.appDirectory);
}

async function writeAll(ctx: Context): Promise<void> {
  const typegenDir = getTypesDir(ctx);

  fs.rmSync(typegenDir, { recursive: true, force: true });
  Object.values(ctx.config.routes).forEach((route) => {
    // We only generate types for routes in the app directory
    if (!isRouteInAppDirectory(ctx, route)) {
      return;
    }

    const typesPath = getTypesPath(ctx, route);
    const content = generate(ctx, route);
    fs.mkdirSync(Path.dirname(typesPath), { recursive: true });
    fs.writeFileSync(typesPath, content);
  });
}


Review comment (suggestion):
`route.file` [is relative to the appDirectory](https://github.com/remix-run/react-router/blob/226dbbdf8f3d6fb1483a995cc321a1ca2a6b882c/packages/react-router-dev/config/routes.ts#L52-L56).

Code after:
  rootDirectory: string,
  { mode, logger }: { mode: string; logger?: vite.Logger }
): Promise<Watcher> {
  const ctx = await createContext({ rootDirectory, mode, watch: true });
  await fs.rm(typesDirectory(ctx), { recursive: true, force: true });
  await write(
    generateFuture(ctx),
    generatePages(ctx),
    generateRoutes(ctx),
    generateServerBuild(ctx),
    ...generateRouteModuleAnnotations(ctx)
  );
  logger?.info(green("generated types"), { timestamp: true, clear: true });

  ctx.configLoader.onChange(
    async ({ result, configChanged, routeConfigChanged }) => {
      if (!result.ok) {
        logger?.error(red(result.error), { timestamp: true, clear: true });
        return;
      }
      ctx.config = result.value;

      if (configChanged) {
        await write(generateFuture(ctx));
        logger?.info(green("regenerated types"), {
          timestamp: true,
          clear: true,
        });
      }

      if (routeConfigChanged) {
        await clearRouteModuleAnnotations(ctx);
        await write(
          generatePages(ctx),
          generateRoutes(ctx),
          ...generateRouteModuleAnnotations(ctx)
        );
        logger?.info(green("regenerated types"), {
          timestamp: true,
          clear: true,
        });
      }
    }
  );

  return {
    close: async () => await ctx.configLoader.close(),
  };
}
Repository: remix-run/react-router
File: packages/react-router-dev/vite/build.ts (TypeScript)

Code before:
    let vite = getVite();
    let ssr = environmentName !== "client";

    let resolveOptions = environmentOptionsResolvers[environmentName];
    invariant(resolveOptions);

    let environmentBuildContext: EnvironmentBuildContext = {
      name: environmentName,
      resolveOptions,
    };

    await vite.build({
      root,
      mode,
      configFile,
      build: {
        assetsInlineLimit,
        emptyOutDir,
        minify,
        ssr,
        sourcemap: ssr ? sourcemapServer : sourcemapClient,
      },
      optimizeDeps: { force },
      clearScreen,
      logLevel,
      ...{ __reactRouterEnvironmentBuildContext: environmentBuildContext },
    });
  }

  let { reactRouterConfig } = ctx;
  let buildManifest = await getBuildManifest(ctx);
  let environmentOptionsResolvers = await getEnvironmentOptionsResolvers(
    ctx,
    buildManifest,
    "build"
  );
  let environmentsOptions = resolveEnvironmentsOptions(
    environmentOptionsResolvers,
    { viteUserConfig }
  );

  await cleanBuildDirectory(viteConfig, ctx);

  // Run the Vite client build first
  await buildEnvironment("client");

  // Then run Vite SSR builds in parallel
  let serverEnvironmentNames = getServerEnvironmentKeys(
    environmentOptionsResolvers,
    buildManifest
  );

Review comment (suggestion):
We now pass the resolved plugin context object into each build so we can re-use the build manifest rather than re-evaluating the `serverBundles` function whenever our Vite plugin is instantiated.

Code after:
    let ssr = environmentName !== "client";

    let resolveOptions = environmentOptionsResolvers[environmentName];
    invariant(resolveOptions);

    let environmentBuildContext: EnvironmentBuildContext = {
      name: environmentName,
      resolveOptions,
    };

    await vite.build({
      root,
      mode,
      configFile,
      build: {
        assetsInlineLimit,
        emptyOutDir,
        minify,
        ssr,
        sourcemap: ssr ? sourcemapServer : sourcemapClient,
      },
      optimizeDeps: { force },
      clearScreen,
      logLevel,
      ...{
        __reactRouterPluginContext: ctx,
        __reactRouterEnvironmentBuildContext: environmentBuildContext,
      },
    });
  }

  let { reactRouterConfig, buildManifest } = ctx;
  invariant(buildManifest, "Expected build manifest to be present");

  let environmentOptionsResolvers = await getEnvironmentOptionsResolvers(
    ctx,
    "build"
  );
  let environmentsOptions = resolveEnvironmentsOptions(
    environmentOptionsResolvers,
    { viteUserConfig }
  );

  await cleanBuildDirectory(viteConfig, ctx);

  // Run the Vite client build first
  await buildEnvironment("client");

  // Then run Vite SSR builds in parallel
  let serverEnvironmentNames = getServerEnvironmentKeys(
    ctx,
Repository: remix-run/react-router
File: packages/react-router/lib/dom/ssr/single-fetch.tsx (TypeScript)

Code before:
  results: SingleFetchResults,
  routeId: string
) {
  let redirect = results[SingleFetchRedirectSymbol];
  if (redirect) {
    return unwrapSingleFetchResult(redirect, routeId);
  }

  return results[routeId] !== undefined
    ? unwrapSingleFetchResult(results[routeId], routeId)
    : null;
}

function unwrapSingleFetchResult(result: SingleFetchResult, routeId: string) {
  if ("error" in result) {
    throw result.error;
  } else if ("redirect" in result) {
    let headers: Record<string, string> = {};
    if (result.revalidate) {
      headers["X-Remix-Revalidate"] = "true";
    }
    if (result.reload) {
      headers["X-Remix-Reload-Document"] = "true";
    }
    if (result.replace) {
      headers["X-Remix-Replace"] = "true";
    }
    throw redirect(result.redirect, { status: result.status, headers });
  } else if ("data" in result) {
    return result.data;
  } else {
    throw new Error(`No response found for routeId "${routeId}"`);
  }
}

function createDeferred<T = unknown>() {
  let resolve: (val?: any) => Promise<void>;
  let reject: (error?: Error) => Promise<void>;
  let promise = new Promise<T>((res, rej) => {
    resolve = async (val: T) => {
      res(val);
      try {
        await promise;
      } catch (e) {}
    };
    reject = async (error?: Error) => {
      rej(error);
      try {
        await promise;
      } catch (e) {}
    };

Review comment (suggestion):
Align these with values used elsewhere - value doesn't really matter since we use `.has(header)`

Code after:
}

function unwrapSingleFetchResult(
  result: DecodedSingleFetchResults,
  routeId: string
) {
  if ("redirect" in result) {
    let {
      redirect: location,
      revalidate,
      reload,
      replace,
      status,
    } = result.redirect;
    throw redirect(location, {
      status,
      headers: {
        // Three R's of redirecting (lol Veep)
        ...(revalidate ? { "X-Remix-Revalidate": "yes" } : null),
        ...(reload ? { "X-Remix-Reload-Document": "yes" } : null),
        ...(replace ? { "X-Remix-Replace": "yes" } : null),
      },
    });
  }

  let routeResult = result.routes[routeId];
  if ("error" in routeResult) {
    throw routeResult.error;
  } else if ("data" in routeResult) {
    return routeResult.data;
  } else {
    throw new Error(`No response found for routeId "${routeId}"`);
  }
}

function createDeferred<T = unknown>() {
  let resolve: (val: T) => Promise<void>;
  let reject: (error: unknown) => Promise<void>;
  let promise = new Promise<T>((res, rej) => {
    resolve = async (val: T) => {
      res(val);
      try {
        await promise;
      } catch (e) {}
    };
    reject = async (error?: unknown) => {
      rej(error);
      try {
        await promise;
      } catch (e) {}
    };
Repository: remix-run/react-router
File: packages/react-router/lib/dom/ssr/errorBoundaries.tsx (TypeScript)

Code before:
  isOutsideRemixApp,
}: {
  error: unknown;
  isOutsideRemixApp?: boolean;
}) {
  console.error(error);

  let heyDeveloper = (
    <script
      dangerouslySetInnerHTML={{
        __html: `
        console.log(
          "💿 Hey developer 👋. You can provide a way better UX than this when your app throws errors. Check out https://reactrouter.com/how-to/error-boundary for more information."
        );
      `,
      }}
    />
  );

  if (isRouteErrorResponse(error)) {
    return (
      <BoundaryShell title="Unhandled Thrown Response!">
        <h1 style={{ fontSize: "24px" }}>
          {error.status} {error.statusText}
        </h1>
        {ENABLE_DEV_WARNINGS && heyDeveloper}
      </BoundaryShell>
    );
  }

  let errorInstance: Error;
  if (error instanceof Error) {
    errorInstance = error;
  } else {
    let errorString =
      error == null
        ? "Unknown Error"
        : typeof error === "object" && "toString" in error
        ? error.toString()
        : JSON.stringify(error);
    errorInstance = new Error(errorString);
  }

  return (
    <BoundaryShell
      title="Application Error!"
      isOutsideRemixApp={isOutsideRemixApp}
    >
      <h1 style={{ fontSize: "24px" }}>Application Error</h1>
      <pre
        style={{

Review comment (suggestion):
Let's [use ternaries rather than `&&` in JSX](https://kentcdodds.com/blog/use-ternaries-rather-than-and-and-in-jsx)

```suggestion
        {ENABLE_DEV_WARNINGS ? heyDeveloper : null}
```

Code after:
  isOutsideRemixApp,
}: {
  error: unknown;
  isOutsideRemixApp?: boolean;
}) {
  console.error(error);

  let heyDeveloper = (
    <script
      dangerouslySetInnerHTML={{
        __html: `
        console.log(
          "💿 Hey developer 👋. You can provide a way better UX than this when your app throws errors. Check out https://reactrouter.com/how-to/error-boundary for more information."
        );
      `,
      }}
    />
  );

  if (isRouteErrorResponse(error)) {
    return (
      <BoundaryShell title="Unhandled Thrown Response!">
        <h1 style={{ fontSize: "24px" }}>
          {error.status} {error.statusText}
        </h1>
        {ENABLE_DEV_WARNINGS ? heyDeveloper : null}
      </BoundaryShell>
    );
  }

  let errorInstance: Error;
  if (error instanceof Error) {
    errorInstance = error;
  } else {
    let errorString =
      error == null
        ? "Unknown Error"
        : typeof error === "object" && "toString" in error
        ? error.toString()
        : JSON.stringify(error);
    errorInstance = new Error(errorString);
  }

  return (
    <BoundaryShell
      title="Application Error!"
      isOutsideRemixApp={isOutsideRemixApp}
    >
      <h1 style={{ fontSize: "24px" }}>Application Error</h1>
      <pre
        style={{
Repository: remix-run/react-router
File: packages/react-router/lib/dom/ssr/fallback.tsx (TypeScript)

Code before:
import * as React from "react";

import { BoundaryShell } from "./errorBoundaries";
import { ENABLE_DEV_WARNINGS } from "../../context";

// If the user sets `clientLoader.hydrate=true` somewhere but does not
// provide a `HydrateFallback` at any level of the tree, then we need to at
// least include `<Scripts>` in the SSR so we can hydrate the app and call the
// `clientLoader` functions
export function RemixRootDefaultHydrateFallback() {
  return (
    <BoundaryShell title="Loading..." renderScripts>
      {ENABLE_DEV_WARNINGS && (
        <script
          dangerouslySetInnerHTML={{
            __html: `
              console.log(
                "💿 Hey developer 👋. You can provide a way better UX than this " +
                "when your app is loading JS modules and/or running \`clientLoader\` " +
                "functions. Check out https://reactrouter.com/start/framework/route-module#hydratefallback " +
                "for more information."
              );
            `,
          }}
        />
      )}
    </BoundaryShell>
  );
}


Review comment (suggestion):
```suggestion
      {ENABLE_DEV_WARNINGS ? (
```

Code after:
import * as React from "react";

import { BoundaryShell } from "./errorBoundaries";
import { ENABLE_DEV_WARNINGS } from "../../context";

// If the user sets `clientLoader.hydrate=true` somewhere but does not
// provide a `HydrateFallback` at any level of the tree, then we need to at
// least include `<Scripts>` in the SSR so we can hydrate the app and call the
// `clientLoader` functions
export function RemixRootDefaultHydrateFallback() {
  return (
    <BoundaryShell title="Loading..." renderScripts>
      {ENABLE_DEV_WARNINGS ? (
        <script
          dangerouslySetInnerHTML={{
            __html: `
              console.log(
                "💿 Hey developer 👋. You can provide a way better UX than this " +
                "when your app is loading JS modules and/or running \`clientLoader\` " +
                "functions. Check out https://reactrouter.com/start/framework/route-module#hydratefallback " +
                "for more information."
              );
            `,
          }}
        />
      ) : null}
    </BoundaryShell>
  );
}
Repository: remix-run/react-router
File: packages/react-router-dev/cli/commands.ts (TypeScript)

Code before:
  let outputEntry = `${entry}.${outputExtension}`;
  let outputFile = path.resolve(appDirectory, outputEntry);

  if (!useTypeScript) {
    let javascript = convertFileToJS(contents, {
      cwd: rootDirectory,
      filename: isServerEntry ? defaultEntryServer : defaultEntryClient,
    });
    await fse.writeFile(outputFile, javascript, "utf-8");
  } else {
    await fse.writeFile(outputFile, contents, "utf-8");
  }

  console.log(
    colors.blue(
      `Entry file ${entry} created at ${path.relative(
        rootDirectory,
        outputFile
      )}.`
    )
  );
}

async function checkForEntry(
  rootDirectory: string,
  appDirectory: string,
  entries: string[]
) {
  for (let entry of entries) {
    let entryPath = path.resolve(appDirectory, entry);
    let exists = await fse.pathExists(entryPath);
    if (exists) {
      let relative = path.relative(rootDirectory, entryPath);
      console.error(colors.red(`Entry file ${relative} already exists.`));
      return process.exit(1);
    }
  }
}

async function createServerEntry(
  rootDirectory: string,
  appDirectory: string,
  inputFile: string
) {
  await checkForEntry(rootDirectory, appDirectory, serverEntries);
  let contents = await fse.readFile(inputFile, "utf-8");
  return contents;
}

async function createClientEntry(
  rootDirectory: string,

Review comment (suggestion):
Note that if a root directory was explicitly provided (e.g. `react-router build my/app/dir`) this will still take precedence. We only fall back to inferring the root when this isn't provided, so anyone relying on this should maintain existing behaviour.

Code after:
  let outputEntry = `${entry}.${outputExtension}`;
  let outputFile = path.resolve(appDirectory, outputEntry);

  if (!useTypeScript) {
    let javascript = convertFileToJS(contents, {
      cwd: rootDirectory,
      filename: isServerEntry ? defaultEntryServer : defaultEntryClient,
    });
    await fse.writeFile(outputFile, javascript, "utf-8");
  } else {
    await fse.writeFile(outputFile, contents, "utf-8");
  }

  console.log(
    colors.blue(
      `Entry file ${entry} created at ${path.relative(
        rootDirectory,
        outputFile
      )}.`
    )
  );
}

function resolveRootDirectory(root?: string, flags?: { config?: string }) {
  if (root) {
    return path.resolve(root);
  }

  return (
    process.env.REACT_ROUTER_ROOT ||
    (flags?.config ? path.dirname(path.resolve(flags.config)) : process.cwd())
  );
}

async function checkForEntry(
  rootDirectory: string,
  appDirectory: string,
  entries: string[]
) {
  for (let entry of entries) {
    let entryPath = path.resolve(appDirectory, entry);
    let exists = await fse.pathExists(entryPath);
    if (exists) {
      let relative = path.relative(rootDirectory, entryPath);
      console.error(colors.red(`Entry file ${relative} already exists.`));
      return process.exit(1);
    }
  }
}

async function createServerEntry(