[remix] Clean up dev symlink upon config error (#9595)

Move the try/catch block to immediately after the symlink call, so that if reading the `remix.config.js` file throws an error or if there's an error parsing the `export const config` in a route, the catch block will clean up after itself properly.

I recommend reviewing with [whitespace changes hidden](https://github.com/vercel/vercel/pull/9595/files?w=1).

packages/remix/src/build.ts

@@ -30,7 +30,7 @@ import type {
   BuildResultV2Typical,
 } from '@vercel/build-utils';
 import type { BaseFunctionConfig } from '@vercel/static-config';
-import type { RemixConfig } from '@remix-run/dev/dist/config';
+import type { AppConfig, RemixConfig } from '@remix-run/dev/dist/config';
 import type { ConfigRoute } from '@remix-run/dev/dist/config/routes';
 import {
   calculateRouteConfigHash,
@@ -111,14 +111,6 @@ export const build: BuildV2 = async ({
     await runNpmInstall(entrypointFsDirname, [], spawnOpts, meta, nodeVersion);
   }
 
-  const remixDevPackageJsonPath = _require.resolve(
-    '@remix-run/dev/package.json',
-    { paths: [entrypointFsDirname] }
-  );
-  const remixVersion = JSON.parse(
-    await fs.readFile(remixDevPackageJsonPath, 'utf8')
-  ).version;
-
   // Make our version of `remix` CLI available to the project's build
   // command by creating a symlink to the copy in our node modules,
   // so that `serverBundles` works: https://github.com/remix-run/remix/pull/5479
@@ -127,110 +119,123 @@ export const build: BuildV2 = async ({
     repoRootPath,
     '@remix-run/dev'
   );
-  const backupRemixRunDevPath = `${remixRunDevPath}.__vercel_backup`;
-  await fs.rename(remixRunDevPath, backupRemixRunDevPath);
-  await fs.symlink(REMIX_RUN_DEV_PATH, remixRunDevPath);
 
-  // Make `remix build` output production mode
+  const remixVersion = JSON.parse(
-  spawnOpts.env.NODE_ENV = 'production';
+    await fs.readFile(join(remixRunDevPath, 'package.json'), 'utf8')
+  ).version;
 
-  const remixConfig = await chdirAndReadConfig(entrypointFsDirname);
+  let remixConfigWrapped = false;
-  const remixRoutes = Object.values(remixConfig.routes);
-
-  // Read the `export const config` (if any) for each route
-  const project = new Project();
-  const staticConfigsMap = new Map<ConfigRoute, BaseFunctionConfig | null>();
-  for (const route of remixRoutes) {
-    const routePath = join(remixConfig.appDirectory, route.file);
-    const staticConfig = getConfig(project, routePath);
-    staticConfigsMap.set(route, staticConfig);
-  }
-
-  const resolvedConfigsMap = new Map<ConfigRoute, ResolvedRouteConfig>();
-  for (const route of remixRoutes) {
-    const config = getResolvedRouteConfig(
-      route,
-      remixConfig.routes,
-      staticConfigsMap
-    );
-    resolvedConfigsMap.set(route, config);
-  }
-
-  // Figure out which routes belong to which server bundles
-  // based on having common static config properties
-  const serverBundlesMap = new Map<string, ConfigRoute[]>();
-  for (const route of remixRoutes) {
-    if (isLayoutRoute(route.id, remixRoutes)) continue;
-
-    const config = resolvedConfigsMap.get(route);
-    if (!config) {
-      throw new Error(`Expected resolved config for "${route.id}"`);
-    }
-    const hash = calculateRouteConfigHash(config);
-
-    let routesForHash = serverBundlesMap.get(hash);
-    if (!Array.isArray(routesForHash)) {
-      routesForHash = [];
-      serverBundlesMap.set(hash, routesForHash);
-    }
-
-    routesForHash.push(route);
-  }
-
-  const serverBundles = Array.from(serverBundlesMap.entries()).map(
-    ([hash, routes]) => {
-      const runtime = resolvedConfigsMap.get(routes[0])?.runtime ?? 'nodejs';
-      return {
-        serverBuildPath: `build/build-${runtime}-${hash}.js`,
-        routes: routes.map(r => r.id),
-      };
-    }
-  );
-
-  // We need to patch the `remix.config.js` file to force some values necessary
-  // for a build that works on either Node.js or the Edge runtime
   const remixConfigPath = findConfig(entrypointFsDirname, 'remix.config');
   const renamedRemixConfigPath = remixConfigPath
     ? `${remixConfigPath}.original${extname(remixConfigPath)}`
     : undefined;
-  if (remixConfigPath && renamedRemixConfigPath) {
-    await fs.rename(remixConfigPath, renamedRemixConfigPath);
 
-    // Figure out if the `remix.config` file is using ESM syntax
+  const backupRemixRunDevPath = `${remixRunDevPath}.__vercel_backup`;
-    let isESM = false;
+  await fs.rename(remixRunDevPath, backupRemixRunDevPath);
-    try {
+  await fs.symlink(REMIX_RUN_DEV_PATH, remixRunDevPath);
-      _require(renamedRemixConfigPath);
+
-    } catch (err: any) {
+  // These get populated inside the try/catch below
-      isESM = err.code === 'ERR_REQUIRE_ESM';
+  let serverBundles: AppConfig['serverBundles'];
+  let remixConfig: RemixConfig;
+  let remixRoutes: ConfigRoute[];
+  const serverBundlesMap = new Map<string, ConfigRoute[]>();
+  const resolvedConfigsMap = new Map<ConfigRoute, ResolvedRouteConfig>();
+
+  try {
+    // Make `remix build` output production mode
+    spawnOpts.env.NODE_ENV = 'production';
+
+    remixConfig = await chdirAndReadConfig(entrypointFsDirname);
+    remixRoutes = Object.values(remixConfig.routes);
+
+    // Read the `export const config` (if any) for each route
+    const project = new Project();
+    const staticConfigsMap = new Map<ConfigRoute, BaseFunctionConfig | null>();
+    for (const route of remixRoutes) {
+      const routePath = join(remixConfig.appDirectory, route.file);
+      const staticConfig = getConfig(project, routePath);
+      staticConfigsMap.set(route, staticConfig);
     }
 
-    let patchedConfig: string;
+    for (const route of remixRoutes) {
-    if (isESM) {
+      const config = getResolvedRouteConfig(
-      patchedConfig = `import config from './${basename(
+        route,
-        renamedRemixConfigPath
+        remixConfig.routes,
-      )}';
+        staticConfigsMap
+      );
+      resolvedConfigsMap.set(route, config);
+    }
+
+    // Figure out which routes belong to which server bundles
+    // based on having common static config properties
+    for (const route of remixRoutes) {
+      if (isLayoutRoute(route.id, remixRoutes)) continue;
+
+      const config = resolvedConfigsMap.get(route);
+      if (!config) {
+        throw new Error(`Expected resolved config for "${route.id}"`);
+      }
+      const hash = calculateRouteConfigHash(config);
+
+      let routesForHash = serverBundlesMap.get(hash);
+      if (!Array.isArray(routesForHash)) {
+        routesForHash = [];
+        serverBundlesMap.set(hash, routesForHash);
+      }
+
+      routesForHash.push(route);
+    }
+
+    serverBundles = Array.from(serverBundlesMap.entries()).map(
+      ([hash, routes]) => {
+        const runtime = resolvedConfigsMap.get(routes[0])?.runtime ?? 'nodejs';
+        return {
+          serverBuildPath: `build/build-${runtime}-${hash}.js`,
+          routes: routes.map(r => r.id),
+        };
+      }
+    );
+
+    // We need to patch the `remix.config.js` file to force some values necessary
+    // for a build that works on either Node.js or the Edge runtime
+    if (remixConfigPath && renamedRemixConfigPath) {
+      await fs.rename(remixConfigPath, renamedRemixConfigPath);
+
+      // Figure out if the `remix.config` file is using ESM syntax
+      let isESM = false;
+      try {
+        _require(renamedRemixConfigPath);
+      } catch (err: any) {
+        isESM = err.code === 'ERR_REQUIRE_ESM';
+      }
+
+      let patchedConfig: string;
+      if (isESM) {
+        patchedConfig = `import config from './${basename(
+          renamedRemixConfigPath
+        )}';
 config.serverBuildTarget = undefined;
 config.serverModuleFormat = 'cjs';
 config.serverPlatform = 'node';
 config.serverBuildPath = undefined;
 config.serverBundles = ${JSON.stringify(serverBundles)};
 export default config;`;
-    } else {
+      } else {
-      patchedConfig = `const config = require('./${basename(
+        patchedConfig = `const config = require('./${basename(
-        renamedRemixConfigPath
+          renamedRemixConfigPath
-      )}');
+        )}');
 config.serverBuildTarget = undefined;
 config.serverModuleFormat = 'cjs';
 config.serverPlatform = 'node';
 config.serverBuildPath = undefined;
 config.serverBundles = ${JSON.stringify(serverBundles)};
 module.exports = config;`;
+      }
+      await fs.writeFile(remixConfigPath, patchedConfig);
+      remixConfigWrapped = true;
     }
-    await fs.writeFile(remixConfigPath, patchedConfig);
-  }
 
-  // Run "Build Command"
+    // Run "Build Command"
-  try {
     if (buildCommand) {
       debug(`Executing build command "${buildCommand}"`);
       await execCommand(buildCommand, {
@@ -260,7 +265,7 @@ module.exports = config;`;
     }
   } finally {
     // Clean up our patched `remix.config.js` to be polite
-    if (remixConfigPath && renamedRemixConfigPath) {
+    if (remixConfigWrapped && remixConfigPath && renamedRemixConfigPath) {
       await fs.rename(renamedRemixConfigPath, remixConfigPath);
     }