wip: tests for transform

remix-run · Nov 12, 2022 · c994670 · c994670
1 parent 9d2cf5b
commit c994670
Show file tree

Hide file tree

Showing 5 changed files with 182 additions and 107 deletions.
diff --git a/packages/remix-dev/__tests__/codemod-replaceRemixMagicImports-test.ts b/packages/remix-dev/__tests__/codemod-replaceRemixMagicImports-test.ts
@@ -1,4 +1,5 @@
 import NpmCliPackageJson from "@npmcli/package-json";
+import fs from "fs";
 import glob from "fast-glob";
 import path from "path";
 import shell from "shelljs";
@@ -12,25 +13,6 @@ import withApp from "./utils/withApp";
 let CODEMOD = "replace-remix-magic-imports";
 let FIXTURE = path.join(__dirname, "fixtures/replace-remix-magic-imports");
 
-// fixture
-// type {}
-// { just values }
-// { just types }
-// { types and values }
-
-// use-case 1: import { a } from "remix"
-// use-case 2: import type { a } from "remix"
-// use-case 3: import { type a } from "remix"
-
-// use-case 4: import { a, b } from "remix"
-// use-case 5: import type { a, b } from "remix"
-// use-case 6: import { type a, type b } from "remix"
-
-// import type { a } from "remix"
-// import { a } from "remix"
-// import { type a } from "remix"
-// import { type a, b } from "remix"
-
 it("replaces `remix` magic imports", async () => {
   await withApp(FIXTURE, async (projectDir) => {
     git.initialCommit(projectDir);
@@ -76,15 +58,31 @@ it("replaces `remix` magic imports", async () => {
 
     // check that `from "remix"` magic imports were removed
     let config = await readConfig(projectDir);
-    let files = glob.sync("**/*.{js,jsx,ts,tsx}", {
+    let files = await glob("**/*.{js,jsx,ts,tsx}", {
       cwd: config.appDirectory,
       absolute: true,
     });
-    let grep = shell.grep("-l", /from ('remix'|"remix")/, files);
-    // let grep = shell.grep(/"stream"/, files);
-    // let grep = shell.grep("-l", 'from "remix"', files);
-    expect(grep.code).toBe(0);
-    expect(grep.stdout.trim()).toBe("");
-    expect(grep.stderr).toBeNull();
+    let remixMagicImports = shell.grep("-l", /from ('remix'|"remix")/, files);
+    expect(remixMagicImports.code).toBe(0);
+    expect(remixMagicImports.stdout.trim()).toBe("");
+    expect(remixMagicImports.stderr).toBeNull();
+
+    // check that imports look good for a specific file
+    let loginRoute = fs.readFileSync(
+      path.join(projectDir, "app/routes/login.tsx"),
+      "utf8"
+    );
+    expect(loginRoute).toContain(
+      [
+        "import {",
+        "  type ActionFunction,",
+        "  type LoaderFunction,",
+        "  type MetaFunction,",
+        "  json,",
+        "  redirect,",
+        '} from "@remix-run/node";',
+        'import { Form, Link, useActionData, useSearchParams } from "@remix-run/react";',
+      ].join("\n")
+    );
   });
 });
diff --git a/packages/remix-dev/__tests__/transform-replaceRemixMagicImports-test.ts b/packages/remix-dev/__tests__/transform-replaceRemixMagicImports-test.ts
@@ -0,0 +1,85 @@
+import replaceRemixMagicImports from "../codemod/replace-remix-magic-imports/transform";
+
+it("replaces single-specifier imports", async () => {
+  let code = [
+    'import { json } from "remix"',
+    'import type { GetLoadContextFunction } from "remix"',
+    'import { type LinkProps } from "remix"',
+  ].join("\n");
+  let transform = replaceRemixMagicImports({
+    runtime: "node",
+    adapter: "express",
+  });
+  let result = transform(code, "fake.tsx");
+  expect(result).toBe(
+    [
+      'import { type GetLoadContextFunction } from "@remix-run/express";',
+      'import { json } from "@remix-run/node";',
+      'import { type LinkProps } from "@remix-run/react";',
+    ].join("\n")
+  );
+});
+
+it("replaces single-kind, multi-specifier imports", async () => {
+  let code = [
+    'import { json, createRequestHandler, Form } from "remix"',
+    'import type { ActionFunction, GetLoadContextFunction, LinkProps } from "remix"',
+    'import { type Cookie, type RequestHandler, type NavLinkProps } from "remix"',
+  ].join("\n");
+  let transform = replaceRemixMagicImports({
+    runtime: "node",
+    adapter: "express",
+  });
+  let result = transform(code, "fake.tsx");
+  expect(result).toBe(
+    [
+      'import { type GetLoadContextFunction, type RequestHandler, createRequestHandler } from "@remix-run/express";',
+      'import { type ActionFunction, type Cookie, json } from "@remix-run/node";',
+      'import { type LinkProps, type NavLinkProps, Form } from "@remix-run/react";',
+    ].join("\n")
+  );
+});
+
+it("replaces multi-kind, multi-specifier imports", async () => {
+  let code = [
+    'import { json, type ActionFunction, createRequestHandler, type GetLoadContextFunction, Form, type LinkProps } from "remix"',
+  ].join("\n");
+  let transform = replaceRemixMagicImports({
+    runtime: "node",
+    adapter: "express",
+  });
+  let result = transform(code, "fake.tsx");
+  expect(result).toBe(
+    [
+      'import { type GetLoadContextFunction, createRequestHandler } from "@remix-run/express";',
+      'import { type ActionFunction, json } from "@remix-run/node";',
+      'import { type LinkProps, Form } from "@remix-run/react";',
+    ].join("\n")
+  );
+});
+
+it("replaces runtime-specific and adapter-specific imports", async () => {
+  let code = [
+    'import { json, createCloudflareKVSessionStorage, createRequestHandler, createPagesFunctionHandler, Form } from "remix"',
+    'import type { ActionFunction, GetLoadContextFunction, createPagesFunctionHandlerParams, LinkProps } from "remix"',
+  ].join("\n");
+  let transform = replaceRemixMagicImports({
+    runtime: "cloudflare",
+    adapter: "cloudflare-pages",
+  });
+  let result = transform(code, "fake.tsx");
+  expect(result).toBe(
+    [
+      'import { type ActionFunction, createCloudflareKVSessionStorage, json } from "@remix-run/cloudflare";',
+      "", // TODO why is this newline here?
+      "import {",
+      "  type GetLoadContextFunction,",
+      "  type createPagesFunctionHandlerParams,",
+      "  createPagesFunctionHandler,",
+      "  createRequestHandler,",
+      '} from "@remix-run/cloudflare-pages";',
+      "", // TODO why is this newline here?
+      'import { type LinkProps, Form } from "@remix-run/react";',
+    ].join("\n")
+  );
+});
diff --git a/packages/remix-dev/__tests__/utils/withApp.ts b/packages/remix-dev/__tests__/utils/withApp.ts
@@ -12,13 +12,13 @@ export default async <Result>(
   );
 
   let projectDir = path.join(TEMP_DIR);
-  // await fse.remove(TEMP_DIR);
+  await fse.remove(TEMP_DIR);
   await fse.ensureDir(TEMP_DIR);
   fse.copySync(fixture, projectDir);
   try {
     let result = await callback(projectDir);
     return result;
   } finally {
-    // await fse.remove(TEMP_DIR);
+    await fse.remove(TEMP_DIR);
   }
 };
diff --git a/packages/remix-dev/codemod/replace-remix-magic-imports/transform.ts b/packages/remix-dev/codemod/replace-remix-magic-imports/transform.ts
@@ -1,5 +1,6 @@
 import { type NodePath } from "@babel/core";
 import * as t from "@babel/types";
+import _ from "lodash";
 
 import createTransform2 from "../createTransform";
 import type { BabelPlugin } from "../utils/babel";
@@ -187,27 +188,34 @@ const plugin =
           );
 
           // group new imports by source
-          let newRemixImportsBySource = groupImportsBySource(newRemixImports);
+          let newRemixImportsBySource: [string, Export[]][] = Array.from(
+            groupImportsBySource(newRemixImports)
+          );
 
           // create new import declarations
-          let newRemixImportDeclarations = Array.from(
-            newRemixImportsBySource
+          let newRemixImportDeclarations = _.sortBy(
+            newRemixImportsBySource,
+            ([source]) => source
           ).map(([source, specifiers]) => {
             return t.importDeclaration(
-              specifiers.map(({ kind, name, alias }) => {
+              _.sortBy(specifiers, ["kind", "name"]).map((spec) => {
+                if (spec.source !== source)
+                  throw Error(
+                    `Specifier source '${spec.source}' does not match declaration source '${source}'`
+                  );
                 return {
                   type: "ImportSpecifier",
-                  local: t.identifier(alias ?? name),
-                  imported: t.identifier(name),
-                  importKind: kind,
+                  local: t.identifier(spec.alias ?? spec.name),
+                  imported: t.identifier(spec.name),
+                  importKind: spec.kind,
                 };
               }),
               t.stringLiteral(source)
             );
           });
 
           // add new remix import declarations
-          currentRemixImportDeclarations[0].insertBefore(
+          currentRemixImportDeclarations[0].insertAfter(
             newRemixImportDeclarations
           );
 

diff --git a/packages/remix-dev/codemod/replace-remix-magic-imports/utils/export.ts b/packages/remix-dev/codemod/replace-remix-magic-imports/utils/export.ts
@@ -34,9 +34,14 @@ const exportsFromNames = <Source extends string = string>(
   ];
 };
 
+type ExportNames = {
+  type: string[];
+  value: string[];
+};
+
 // Runtimes
 
-const defaultRuntimeExports = {
+const defaultRuntimeExports: ExportNames = {
   type: [
     "ActionFunction",
     "AppData",
@@ -91,96 +96,75 @@ const defaultRuntimeExports = {
     "unstable_createMemoryUploadHandler",
     "unstable_parseMultipartFormData",
   ],
-} as const;
+};
 
-const toRuntimeExports = (
-  runtime: Runtime,
-  names: {
-    type?: string[];
-    value?: string[];
-  } = {}
-) => {
+const exportNamesByRuntime: Record<Runtime, Partial<ExportNames>> = {
+  cloudflare: {
+    value: ["createCloudflareKVSessionStorage"],
+  },
+  node: {
+    type: ["HeadersInit", "RequestInfo", "RequestInit", "ResponseInit"],
+    value: [
+      "AbortController",
+      "createFileSessionStorage",
+      "createReadableStreamFromReadable",
+      "fetch",
+      "FormData",
+      "Headers",
+      "installGlobals",
+      "NodeOnDiskFile",
+      "readableStreamToString",
+      "Request",
+      "Response",
+      "unstable_createFileUploadHandler",
+      "writeAsyncIterableToWritable",
+      "writeReadableStreamToWritable",
+    ],
+  },
+};
+
+export const getRuntimeExports = (runtime: Runtime) => {
+  let names = exportNamesByRuntime[runtime];
   return exportsFromNames(`@remix-run/${runtime}`, {
     type: [...defaultRuntimeExports.type, ...(names.type ?? [])],
     value: [...defaultRuntimeExports.value, ...(names.value ?? [])],
   });
 };
 
-const exportsByRuntime: Record<Runtime, { type?: string[]; value?: string[] }> =
-  {
-    cloudflare: {
-      value: ["createCloudflareKVSessionStorage"],
-    },
-    node: {
-      type: ["HeadersInit", "RequestInfo", "RequestInit", "ResponseInit"],
-      value: [
-        "AbortController",
-        "createFileSessionStorage",
-        "createReadableStreamFromReadable",
-        "fetch",
-        "FormData",
-        "Headers",
-        "installGlobals",
-        "NodeOnDiskFile",
-        "readableStreamToString",
-        "Request",
-        "Response",
-        "unstable_createFileUploadHandler",
-        "writeAsyncIterableToWritable",
-        "writeReadableStreamToWritable",
-      ],
-    },
-  };
-
-export const getRuntimeExports = (runtime: Runtime) =>
-  toRuntimeExports(runtime, exportsByRuntime[runtime]);
-
 // Adapters
 
-const defaultAdapterExports = {
+const defaultAdapterExports: ExportNames = {
   type: ["GetLoadContextFunction", "RequestHandler"],
   value: ["createRequestHandler"],
-} as const;
+};
 
-const toAdapterExports = (
-  adapter: Adapter,
-  names: {
-    type?: string[];
-    value?: string[];
-  } = {}
-) => {
+const exportNamesByAdapter: Record<Adapter, Partial<ExportNames>> = {
+  architect: {
+    value: ["createArcTableSessionStorage"],
+  },
+  "cloudflare-pages": {
+    type: ["createPagesFunctionHandlerParams"],
+    value: ["createPagesFunctionHandler"],
+  },
+  "cloudflare-workers": {
+    value: ["createEventHandler", "handleAsset"],
+  },
+  express: {},
+  netlify: {},
+  vercel: {},
+};
+
+export const getAdapterExports = (adapter: Adapter) => {
+  let names = exportNamesByAdapter[adapter];
   return exportsFromNames(`@remix-run/${adapter}`, {
     type: [...defaultAdapterExports.type, ...(names.type ?? [])],
     value: [...defaultAdapterExports.value, ...(names.value ?? [])],
   });
 };
 
-const exportsByAdapter: Record<Adapter, { type?: string[]; value?: string[] }> =
-  {
-    architect: {
-      value: ["createArcTableSessionStorage"],
-    },
-    "cloudflare-pages": {
-      type: ["createPagesFunctionHandlerParams"],
-      value: ["createPagesFunctionHandler"],
-    },
-    "cloudflare-workers": {
-      value: ["createEventHandler", "handleAsset"],
-    },
-    express: {},
-    netlify: {},
-    vercel: {},
-  };
-
-export const getAdapterExports = (adapter: Adapter) =>
-  toAdapterExports(adapter, exportsByAdapter[adapter]);
-
 // Renderers
 
-const exportsByRenderer: Record<
-  Renderer,
-  { type?: string[]; value?: string[] }
-> = {
+const exportsByRenderer: Record<Renderer, Partial<ExportNames>> = {
   react: {
     type: [
       "FormEncType",