fix(2.0): niche cases for DCE

apps/tests/src/e2e/server-function.test.ts

@@ -91,4 +91,18 @@ test.describe("server-function", () => {
     await page.goto("http://localhost:3000/server-env");
     await expect(page.locator("#server-fn-test")).toContainText('{"result":true}');
   });
+
+  test("should build with a server function including an unused try/catch variable", async ({
+    page,
+  }) => {
+    await page.goto("http://localhost:3000/server-function-unused-trycatch");
+    await expect(page.locator("#server-fn-test")).toContainText("false");
+  });
+
+  test("should build with a server function including an unused destructured variable", async ({
+    page,
+  }) => {
+    await page.goto("http://localhost:3000/server-function-unused-destructure");
+    await expect(page.locator("#server-fn-test")).toContainText("false");
+  });
 });

apps/tests/src/routes/server-function-unused-destructure.tsx

@@ -0,0 +1,28 @@
+import { createEffect, createSignal } from "solid-js";
+
+function serverFnDestructure() {
+  "use server";
+
+  const rawItems = [{ id: "", age: 42 }];
+  const items: { age: number }[] = [];
+  for (const { id, ...rest } of rawItems) {
+    items.push(rest);
+  }
+
+  return false;
+}
+
+export default function App() {
+  const [output, setOutput] = createSignal<boolean>();
+
+  createEffect(async () => {
+    const result = await serverFnDestructure();
+    setOutput(result);
+  });
+
+  return (
+    <main>
+      <span id="server-fn-test">{JSON.stringify(output())}</span>
+    </main>
+  );
+}

apps/tests/src/routes/server-function-unused-trycatch.tsx

@@ -0,0 +1,26 @@
+import { createEffect, createSignal } from "solid-js";
+
+function serverFnTryCatch() {
+  "use server";
+
+  try {
+    throw new Error();
+  } catch (error) {
+    return false;
+  }
+}
+
+export default function App() {
+  const [output, setOutput] = createSignal<boolean>();
+
+  createEffect(async () => {
+    const result = await serverFnTryCatch();
+    setOutput(result);
+  });
+
+  return (
+    <main>
+      <span id="server-fn-test">{JSON.stringify(output())}</span>
+    </main>
+  );
+}

packages/start/src/config/index.ts

@@ -63,6 +63,18 @@ export function solidStart(options?: SolidStartOptions): Array<PluginOption> {
     server: `${start.appRoot}/entry-server${entryExtension}`,
   };
   return [
+    // TODO (Alexis): check if the comment below is still relevant
+    //
+    // Must be placed after fsRoutes, as treeShake will remove the
+    // server fn exports added in by this plugin
+    serverFunctionsPlugin({
+      manifest: VIRTUAL_MODULES.serverFnManifest,
+      runtime: {
+        server: '@solidjs/start/fns/server',
+        client: '@solidjs/start/fns/client',
+      },
+      filter: options?.serverFunctions?.filter,
+    }),
     {
       name: "solid-start:config",
       enforce: "pre",
@@ -190,16 +202,6 @@ export function solidStart(options?: SolidStartOptions): Array<PluginOption> {
     }),
     lazy(),
     envPlugin(options?.env),
-    // Must be placed after fsRoutes, as treeShake will remove the
-    // server fn exports added in by this plugin
-    serverFunctionsPlugin({
-      manifest: VIRTUAL_MODULES.serverFnManifest,
-      runtime: {
-        server: '@solidjs/start/fns/server',
-        client: '@solidjs/start/fns/client',
-      },
-      filter: options?.serverFunctions?.filter,
-    }),
     {
       name: "solid-start:virtual-modules",
       async resolveId(id) {

packages/start/src/directives/index.ts

@@ -196,6 +196,7 @@ export function serverFunctionsPlugin(options: ServerFunctionsOptions): Plugin[]
     },
     {
       name: "solid-start:server-functions/compiler",
+      enforce: 'pre',
       async transform(code, fileId, opts) {
         const mode = opts?.ssr ? "server" : "client";
         const [id] = fileId.split("?");

packages/start/src/directives/remove-unused-variables.ts

@@ -2,6 +2,20 @@ import type * as babel from "@babel/core";
 import * as t from "@babel/types";
 import { isPathValid } from "./paths.ts";
 
+function isInvalidForRemoval(path: babel.NodePath) {
+  if (isPathValid(path, t.isCatchClause)) {
+    // This case is for `catch (error)` blocks
+    return true;
+  }
+
+  // This one is for destructured variables
+  let target = path;
+  if (isPathValid(path, t.isVariableDeclarator)) {
+    target = path.get('id');
+  }
+  return isPathValid(target, t.isObjectPattern) || isPathValid(target, t.isArrayPattern);
+}
+
 export function removeUnusedVariables(program: babel.NodePath<t.Program>) {
   // TODO(Alexis):
   // This implementation is simple but slow
@@ -24,17 +38,18 @@ export function removeUnusedVariables(program: babel.NodePath<t.Program>) {
             case "hoisted":
             case "module":
               if (binding.references === 0 && !binding.path.removed) {
-                if (isPathValid(binding.path.parentPath, t.isImportDeclaration)) {
                   const parent = binding.path.parentPath;
+                if (isPathValid(parent, t.isImportDeclaration)) {
                   if (parent.node.specifiers.length === 1) {
                     parent.remove();
                   } else {
                     binding.path.remove();
                   }
-                } else {
+                  dirty = true;
+                } else if (!(isInvalidForRemoval(binding.path))) {
                   binding.path.remove();
+                  dirty = true;
                 }
-                dirty = true;
               }
               break;
             case "local":