Repository: remix-run/remix
Pull request #3529 (2022-06-21 12:22:16)

refactor(remix-dev/cli): use `convert-to-javascript` migration internally
Follow-up of #3150

changeset-bot[bot]: ###  ⚠️  No Changeset found

Latest commit: 210e8dc8013848cac0644196c08b5c9ad840a8a9

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. **If these changes should result in a version bump, you need to add a changeset.**

<details><summary>This PR includes no changesets</summary>

  When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

</details>

[Click here to learn what changesets are, and how to add one](https://github.com/changesets/changesets/blob/master/docs/adding-a-changeset.md).

[Click here if you're a maintainer who wants to add a changeset to this PR](https://github.com/MichaelDeBoey/remix/new/use-convert-to-javascript-migration-internally?filename=.changeset/big-eels-guess.md&value=---%0A%22remix%22%3A%20patch%0A%22%40remix-run%2Fdev%22%3A%20patch%0A---%0A%0Arefactor(remix-dev%2Fcli)%3A%20use%20%60convert-to-javascript%60%20migration%20internally%0A)


pcattori: @MichaelDeBoey I think the best way to proceed is to introduce a `--interactive` flag that to the CLI that defaults to `require.main === module`, but can be set explicitly by users or by tests. Note that we [aren't catering to the test user](https://kentcdodds.com/blog/avoid-the-test-user), since _actual_ users may also want to set `--interactive=false` in their automation.
MichaelDeBoey: @kentcdodds @pcattori `--interactive` & `--no-interactive` flags are implemented in the `convert-to-javascript` migration now 👍

Patch:
diff --git a/packages/remix-dev/cli/convert-to-javascript.ts b/packages/remix-dev/cli/convert-to-javascript.ts
deleted file mode 100644
index 59f7dbfe9..000000000
--- a/packages/remix-dev/cli/convert-to-javascript.ts
+++ /dev/null
@@ -1,85 +0,0 @@
-import path from "path";
-import fse from "fs-extra";
-import glob from "fast-glob";
-import * as babel from "@babel/core";
-// @ts-expect-error these modules dont have types
-import babelPluginSyntaxJSX from "@babel/plugin-syntax-jsx";
-// @ts-expect-error these modules dont have types
-import babelPresetTypeScript from "@babel/preset-typescript";
-import prettier from "prettier";
-
-function convertToJavaScript(
-  filename: string,
-  source: string,
-  projectDir: string
-): string {
-  let result = babel.transformSync(source, {
-    filename,
-    presets: [[babelPresetTypeScript, { jsx: "preserve" }]],
-    plugins: [babelPluginSyntaxJSX],
-    compact: false,
-    retainLines: true,
-    cwd: projectDir,
-  });
-
-  if (!result || !result.code) {
-    throw new Error("Could not parse typescript");
-  }
-
-  /*
-    Babel's `compact` and `retainLines` options are both bad at formatting code.
-    Use Prettier for nicer formatting.
-  */
-  return prettier.format(result.code, { parser: "babel" });
-}
-
-export async function convertTemplateToJavaScript(projectDir: string) {
-  // 1. Convert all .ts files in the template to .js
-  let entries = glob.sync("**/*.+(ts|tsx)", {
-    cwd: projectDir,
-    absolute: true,
-    ignore: ["**/node_modules/**"],
-  });
-  for (let entry of entries) {
-    if (entry.endsWith(".d.ts")) {
-      fse.removeSync(entry);
-      continue;
-    }
-
-    let contents = fse.readFileSync(entry, "utf8");
-    let filename = path.basename(entry);
-    let javascript = convertToJavaScript(filename, contents, projectDir);
-
-    fse.writeFileSync(entry, javascript, "utf8");
-    if (entry.endsWith(".tsx")) {
-      fse.renameSync(entry, entry.replace(/\.tsx?$/, ".jsx"));
-    } else {
-      fse.renameSync(entry, entry.replace(/\.ts?$/, ".js"));
-    }
-  }
-
-  // 2. Rename the tsconfig.json to jsconfig.json
-  if (fse.existsSync(path.join(projectDir, "tsconfig.json"))) {
-    fse.renameSync(
-      path.join(projectDir, "tsconfig.json"),
-      path.join(projectDir, "jsconfig.json")
-    );
-  }
-
-  // 3. Remove @types/* and typescript from package.json
-  let packageJsonPath = path.join(projectDir, "package.json");
-  if (!fse.existsSync(packageJsonPath)) {
-    throw new Error("Could not find package.json");
-  }
-  let pkg = JSON.parse(fse.readFileSync(packageJsonPath, "utf8"));
-  let devDeps = pkg.devDependencies || {};
-  devDeps = Object.fromEntries(
-    Object.entries(devDeps).filter(([name]) => {
-      return !name.startsWith("@types/") && name !== "typescript";
-    })
-  );
-  pkg.devDependencies = devDeps;
-  fse.writeJSONSync(packageJsonPath, pkg, {
-    spaces: 2,
-  });
-}
diff --git a/packages/remix-dev/cli/create.ts b/packages/remix-dev/cli/create.ts
index 008aa7b6d..de0fbbb18 100644
--- a/packages/remix-dev/cli/create.ts
+++ b/packages/remix-dev/cli/create.ts
@@ -1,21 +1,21 @@
+import { execSync } from "child_process";
+import path from "path";
 import stream from "stream";
+import { fileURLToPath } from "url";
 import { promisify } from "util";
-import path from "path";
 import fse from "fs-extra";
+import gunzip from "gunzip-maybe";
 import fetch from "node-fetch";
 import ora from "ora";
-import gunzip from "gunzip-maybe";
-import tar from "tar-fs";
 import * as semver from "semver";
-import { fileURLToPath } from "url";
-import { execSync } from "child_process";
 import sortPackageJSON from "sort-package-json";
+import tar from "tar-fs";
 
 import * as colors from "../colors";
+import invariant from "../invariant";
 import packageJson from "../package.json";
-import { convertTemplateToJavaScript } from "./convert-to-javascript";
 import { getPreferredPackageManager } from "./getPreferredPackageManager";
-import invariant from "../invariant";
+import { convertToJavaScript } from "./migrate/migrations/convert-to-javascript";
 
 const remixDevPackageVersion = packageJson.version;
 
@@ -196,8 +196,8 @@ export async function createApp({
     !useTypeScript &&
     fse.existsSync(path.join(projectDir, "tsconfig.json"))
   ) {
-    let spinner = ora("Converting template to JavaScript…").start();
-    await convertTemplateToJavaScript(projectDir);
+    let spinner = ora("Migrating template to JavaScript…").start();
+    await convertToJavaScript(projectDir, { interactive: false });
     spinner.stop();
     spinner.clear();
   }
diff --git a/packages/remix-dev/cli/migrate/flags.ts b/packages/remix-dev/cli/migrate/flags.ts
index 94e60d7b2..4e2857a7f 100644
--- a/packages/remix-dev/cli/migrate/flags.ts
+++ b/packages/remix-dev/cli/migrate/flags.ts
@@ -2,6 +2,7 @@ export interface Flags {
   debug?: boolean;
   dry?: boolean;
   force?: boolean;
+  interactive?: boolean;
   print?: boolean;
   runInBand?: boolean;
 }
diff --git a/packages/remix-dev/cli/migrate/migrations/convert-to-javascript/convertTSFilesToJS/index.ts b/packages/remix-dev/cli/migrate/migrations/convert-to-javascript/convertTSFilesToJS/index.ts
index ef1f288ea..f7a2581d0 100644
--- a/packages/remix-dev/cli/migrate/migrations/convert-to-javascript/convertTSFilesToJS/index.ts
+++ b/packages/remix-dev/cli/migrate/migrations/convert-to-javascript/convertTSFilesToJS/index.ts
@@ -6,8 +6,8 @@ import { convertTSFileToJS } from "./convertTSFileToJS";
 
 export const convertTSFilesToJS = (projectDir: string) => {
   let entries = glob.sync("**/*.+(ts|tsx)", {
-    cwd: projectDir,
     absolute: true,
+    cwd: projectDir,
     ignore: ["**/node_modules/**"],
   });
 
diff --git a/packages/remix-dev/cli/migrate/migrations/convert-to-javascript/index.ts b/packages/remix-dev/cli/migrate/migrations/convert-to-javascript/index.ts
index 8a3c6a419..8b89feed5 100644
--- a/packages/remix-dev/cli/migrate/migrations/convert-to-javascript/index.ts
+++ b/packages/remix-dev/cli/migrate/migrations/convert-to-javascript/index.ts
@@ -10,16 +10,16 @@ import { convertTSFilesToJS } from "./convertTSFilesToJS";
 
 const TRANSFORM_PATH = join(__dirname, "transform");
 
-export const convertToJavaScript: MigrationFunction = async ({
-  flags,
+export const convertToJavaScript: MigrationFunction = async (
   projectDir,
-}) => {
+  flags = {}
+) => {
   let config = await readConfig(projectDir);
 
   // 1. Rename all tsconfig.json files to jsconfig.json
   convertTSConfigs(config.rootDirectory);
 
-  // 2. Remove @types/* & TypeScript dependencies + typecheck script from package.json
+  // 2. Remove @types/* & TypeScript dependencies + `typecheck` script from `package.json`
   await cleanupPackageJson(config.rootDirectory);
 
   // 3. Run codemod
@@ -35,15 +35,19 @@ export const convertToJavaScript: MigrationFunction = async ({
   });
   if (!codemodOk) {
     console.error("❌ I couldn't update your imports to JS.");
-    if (!flags.debug) {
+
+    if (flags.interactive && !flags.debug) {
       console.log("👉 Try again with the `--debug` flag to see what failed.");
     }
+
     process.exit(1);
   }
 
   // 4. Convert all .ts files to .js
   convertTSFilesToJS(config.rootDirectory);
-  console.log("✅ Your JavaScript looks good!");
+  if (flags.interactive) {
+    console.log("✅ Your JavaScript looks good!");
 
-  console.log("\n🚚 I've successfully migrated your project! 🎉");
+    console.log("\n🚚 I've successfully migrated your project! 🎉");
+  }
 };
diff --git a/packages/remix-dev/cli/migrate/migrations/replace-remix-imports/index.ts b/packages/remix-dev/cli/migrate/migrations/replace-remix-imports/index.ts
index dbd32dd46..52950d747 100644
--- a/packages/remix-dev/cli/migrate/migrations/replace-remix-imports/index.ts
+++ b/packages/remix-dev/cli/migrate/migrations/replace-remix-imports/index.ts
@@ -35,11 +35,14 @@ const getRemixVersionSpec = (remixDeps: Dependency[]): string => {
     console.error("❌ I couldn't find versions for your Remix packages.");
     process.exit(1);
   }
+
   if (semver.lt(candidateMin, "1.3.3")) {
     console.log("⬆️  I'm upgrading your Remix dependencies");
     console.log(because("this migration requires v1.3.3 or newer."));
+
     return "^1.3.3";
   }
+
   console.log(
     detected(
       `\`${colors.blue(
@@ -52,7 +55,9 @@ const getRemixVersionSpec = (remixDeps: Dependency[]): string => {
 };
 
 const shouldKeepPostinstall = (original?: string): boolean => {
-  if (original === undefined) return false;
+  if (original === undefined) {
+    return false;
+  }
 
   if (onlyRemixSetup.test(original) || onlyRemixSetupRuntime.test(original)) {
     console.log(
@@ -75,10 +80,10 @@ const shouldKeepPostinstall = (original?: string): boolean => {
   return true;
 };
 
-export const replaceRemixImports: MigrationFunction = async ({
-  flags,
+export const replaceRemixImports: MigrationFunction = async (
   projectDir,
-}) => {
+  flags = {}
+) => {
   let pkg = await NpmCliPackageJson.load(projectDir);
 
   // 0. resolve runtime and adapter
diff --git a/packages/remix-dev/cli/migrate/run.ts b/packages/remix-dev/cli/migrate/run.ts
index 0b48578fb..a9c748c1f 100644
--- a/packages/remix-dev/cli/migrate/run.ts
+++ b/packages/remix-dev/cli/migrate/run.ts
@@ -7,12 +7,14 @@ import type { Migration } from "./types";
 
 const parseMigration = (migrationId: string): Migration => {
   let migration = migrations.find(({ id }) => id === migrationId);
+
   if (migration === undefined) {
     throw Error(`
 ${colors.error("Invalid migration. Pick one of:")}
 ${migrations.map((m) => colors.error(`- ${m.id}`)).join("\n")}
     `);
   }
+
   return migration;
 };
 
@@ -20,9 +22,11 @@ const checkProjectDir = (projectDir: string): string => {
   if (!fse.existsSync(projectDir)) {
     throw Error(`Project path does not exist: ${projectDir}`);
   }
+
   if (!fse.lstatSync(projectDir).isDirectory()) {
     throw Error(`Project path is not a directory: ${projectDir}`);
   }
+
   return projectDir;
 };
 
@@ -34,5 +38,5 @@ export const run = async (input: {
   let projectDir = checkProjectDir(input.projectDir);
   let migration = parseMigration(input.migrationId);
 
-  return migration.function({ flags: input.flags, projectDir });
+  return migration.function(projectDir, input.flags);
 };
diff --git a/packages/remix-dev/cli/migrate/types.ts b/packages/remix-dev/cli/migrate/types.ts
index 89a79fb54..d4e2b57cb 100644
--- a/packages/remix-dev/cli/migrate/types.ts
+++ b/packages/remix-dev/cli/migrate/types.ts
@@ -1,9 +1,9 @@
 import type { Flags } from "./flags";
 
-export type MigrationFunction = (args: {
-  flags: Flags;
-  projectDir: string;
-}) => Promise<void>;
+export type MigrationFunction = (
+  projectDir: string,
+  flags?: Flags
+) => Promise<void>;
 
 export interface Migration {
   id: string;
diff --git a/packages/remix-dev/cli/run.ts b/packages/remix-dev/cli/run.ts
index a6b8bcda6..b2e64293b 100644
--- a/packages/remix-dev/cli/run.ts
+++ b/packages/remix-dev/cli/run.ts
@@ -160,6 +160,8 @@ export async function run(argv: string[] = process.argv.slice(2)) {
       "-h": "--help",
       "--install": Boolean,
       "--no-install": Boolean,
+      "--interactive": Boolean,
+      "--no-interactive": Boolean,
       "--json": Boolean,
       "--migration": String,
       "-m": "--migration",
@@ -203,6 +205,10 @@ export async function run(argv: string[] = process.argv.slice(2)) {
   if (args["--no-install"]) {
     flags.install = false;
   }
+  if (args["--no-interactive"]) {
+    flags.interactive = false;
+  }
+  flags.interactive ??= require.main === module;
   if (args["--no-typescript"]) {
     flags.typescript = false;
   }


Test patch:
diff --git a/packages/remix-dev/__tests__/cli-test.ts b/packages/remix-dev/__tests__/cli-test.ts
index 3fafd540f..979617aea 100644
--- a/packages/remix-dev/__tests__/cli-test.ts
+++ b/packages/remix-dev/__tests__/cli-test.ts
@@ -1,8 +1,8 @@
 import childProcess from "child_process";
-import fse from "fs-extra";
 import os from "os";
 import path from "path";
 import util from "util";
+import fse from "fs-extra";
 import semver from "semver";
 
 let DOWN = "\x1B\x5B\x42";
@@ -235,23 +235,16 @@ describe("remix CLI", () => {
         { question: /install/i, type: ["n", ENTER] },
       ]);
 
-      expect(
-        fse.existsSync(path.join(projectDir, "package.json"))
-      ).toBeTruthy();
+      expect(fse.existsSync(path.join(projectDir, "app/root.tsx"))).toBeFalsy();
       expect(
         fse.existsSync(path.join(projectDir, "app/root.jsx"))
       ).toBeTruthy();
-      expect(fse.existsSync(path.join(projectDir, "app/root.tsx"))).toBeFalsy();
       expect(
         fse.existsSync(path.join(projectDir, "tsconfig.json"))
       ).toBeFalsy();
       expect(
         fse.existsSync(path.join(projectDir, "jsconfig.json"))
       ).toBeTruthy();
-      let pkgJSON = JSON.parse(
-        fse.readFileSync(path.join(projectDir, "package.json"), "utf-8")
-      );
-      expect(Object.keys(pkgJSON.devDependencies)).not.toContain("typescript");
     });
   });
 });
diff --git a/packages/remix-dev/__tests__/create-test.ts b/packages/remix-dev/__tests__/create-test.ts
index eb8897242..4e8208cdc 100644
--- a/packages/remix-dev/__tests__/create-test.ts
+++ b/packages/remix-dev/__tests__/create-test.ts
@@ -1,10 +1,10 @@
 import { execSync } from "child_process";
-import fse from "fs-extra";
-import os from "os";
-import path from "path";
+import * as os from "os";
+import * as path from "path";
 import { pathToFileURL } from "url";
-import stripAnsi from "strip-ansi";
+import * as fse from "fs-extra";
 import inquirer from "inquirer";
+import stripAnsi from "strip-ansi";
 
 import { run } from "../cli/run";
 import { server } from "./msw";
@@ -336,7 +336,7 @@ describe("the create command", () => {
     expect(fse.existsSync(path.join(projectDir, "app/root.tsx"))).toBeTruthy();
   });
 
-  it("converts a template to javascript", async () => {
+  it("converts a template to JavaScript", async () => {
     let projectDir = await getProjectDir("template-to-js");
     await run([
       "create",
@@ -351,16 +351,10 @@ describe("the create command", () => {
         "\n\n" +
         getSuccessMessage(path.join("<TEMP_DIR>", "template-to-js"))
     );
-    expect(fse.existsSync(path.join(projectDir, "package.json"))).toBeTruthy();
-    expect(fse.existsSync(path.join(projectDir, "app/root.jsx"))).toBeTruthy();
     expect(fse.existsSync(path.join(projectDir, "app/root.tsx"))).toBeFalsy();
+    expect(fse.existsSync(path.join(projectDir, "app/root.jsx"))).toBeTruthy();
     expect(fse.existsSync(path.join(projectDir, "tsconfig.json"))).toBeFalsy();
     expect(fse.existsSync(path.join(projectDir, "jsconfig.json"))).toBeTruthy();
-    expect(fse.existsSync(path.join(projectDir, "app/utils.js"))).toBeTruthy();
-    let pkgJSON = JSON.parse(
-      fse.readFileSync(path.join(projectDir, "package.json"), "utf-8")
-    );
-    expect(Object.keys(pkgJSON.devDependencies)).not.toContain("typescript");
   });
 
   it("works for a file path to a directory on disk", async () => {
diff --git a/packages/remix-dev/__tests__/migrations/convert-to-javascript-test.ts b/packages/remix-dev/__tests__/migrations/convert-to-javascript-test.ts
index d4e8e4fbe..0fc1c3339 100644
--- a/packages/remix-dev/__tests__/migrations/convert-to-javascript-test.ts
+++ b/packages/remix-dev/__tests__/migrations/convert-to-javascript-test.ts
@@ -1,5 +1,6 @@
+import { spawnSync } from "child_process";
 import { tmpdir } from "os";
-import { join } from "path";
+import { join, resolve } from "path";
 import glob from "fast-glob";
 import {
   copySync,
@@ -60,6 +61,40 @@ afterEach(() => {
   removeSync(TEMP_DIR);
 });
 
+const checkMigrationRanSuccessfully = async (projectDir: string) => {
+  let config = await readConfig(projectDir);
+
+  let jsConfigJson: TsConfigJson = readJSONSync(
+    join(projectDir, "jsconfig.json")
+  );
+  expect(jsConfigJson.include).not.toContain("remix.env.d.ts");
+  expect(jsConfigJson.include).not.toContain("**/*.ts");
+  expect(jsConfigJson.include).toContain("**/*.js");
+  expect(jsConfigJson.include).not.toContain("**/*.tsx");
+  expect(jsConfigJson.include).toContain("**/*.jsx");
+
+  let packageJson: PackageJson = readJSONSync(join(projectDir, "package.json"));
+  expect(packageJson.devDependencies).not.toContain("@types/react");
+  expect(packageJson.devDependencies).not.toContain("@types/react-dom");
+  expect(packageJson.devDependencies).not.toContain("typescript");
+  expect(packageJson.scripts).not.toContain("typecheck");
+
+  let TSFiles = glob.sync("**/*.@(ts|tsx)", {
+    cwd: config.rootDirectory,
+    ignore: [`./${config.appDirectory}/**/*`],
+  });
+  expect(TSFiles).toHaveLength(0);
+  let JSFiles = glob.sync("**/*.@(js|jsx)", {
+    absolute: true,
+    cwd: config.rootDirectory,
+    ignore: [`./${config.appDirectory}/**/*`],
+  });
+  let result = shell.grep("-l", 'from "', JSFiles);
+  expect(result.stdout.trim()).toBe("");
+  expect(result.stderr).toBeNull();
+  expect(result.code).toBe(0);
+};
+
 const makeApp = () => {
   let projectDir = join(TEMP_DIR, "convert-to-javascript");
 
@@ -68,56 +103,50 @@ const makeApp = () => {
   return projectDir;
 };
 
-const runConvertToJavaScriptMigration = (projectDir: string) =>
-  run([
-    "migrate",
-    "--migration",
-    "convert-to-javascript",
-    projectDir,
-    "--force",
-  ]);
+const getRunArgs = (projectDir: string) => [
+  "migrate",
+  "--migration",
+  "convert-to-javascript",
+  projectDir,
+  "--force",
+];
+const runConvertToJavaScriptMigrationProgrammatically = (projectDir: string) =>
+  run([...getRunArgs(projectDir), "--no-interactive"]);
+const runConvertToJavaScriptMigrationViaCLI = (projectDir: string) =>
+  spawnSync(
+    "node",
+    [
+      "--require",
+      require.resolve("esbuild-register"),
+      "--require",
+      join(__dirname, "..", "msw.ts"),
+      resolve(__dirname, "..", "..", "cli.ts"),
+      ...getRunArgs(projectDir),
+      "--interactive",
+    ],
+    { cwd: projectDir }
+  ).stdout?.toString("utf-8");
 
 describe("`convert-to-javascript` migration", () => {
-  it("runs successfully", async () => {
+  it("runs successfully when ran via CLI", async () => {
     let projectDir = makeApp();
-    let config = await readConfig(projectDir);
 
-    await runConvertToJavaScriptMigration(projectDir);
+    let output = runConvertToJavaScriptMigrationViaCLI(projectDir);
 
-    let jsConfigJson: TsConfigJson = readJSONSync(
-      join(projectDir, "jsconfig.json")
-    );
-    expect(jsConfigJson.include).not.toContain("remix.env.d.ts");
-    expect(jsConfigJson.include).not.toContain("**/*.ts");
-    expect(jsConfigJson.include).toContain("**/*.js");
-    expect(jsConfigJson.include).not.toContain("**/*.tsx");
-    expect(jsConfigJson.include).toContain("**/*.jsx");
-
-    let packageJson: PackageJson = readJSONSync(
-      join(projectDir, "package.json")
-    );
-    expect(packageJson.devDependencies).not.toContain("@types/react");
-    expect(packageJson.devDependencies).not.toContain("@types/react-dom");
-    expect(packageJson.devDependencies).not.toContain("typescript");
-    expect(packageJson.scripts).not.toContain("typecheck");
+    await checkMigrationRanSuccessfully(projectDir);
 
     expect(output).toContain("✅ Your JavaScript looks good!");
+    expect(output).toContain("successfully migrated");
+  });
+
+  it("runs successfully when ran programmatically", async () => {
+    let projectDir = makeApp();
 
-    let TSFiles = glob.sync("**/*.@(ts|tsx)", {
-      cwd: config.rootDirectory,
-      ignore: [`./${config.appDirectory}/**/*`],
-    });
-    expect(TSFiles).toHaveLength(0);
-    let JSFiles = glob.sync("**/*.@(js|jsx)", {
-      absolute: true,
-      cwd: config.rootDirectory,
-      ignore: [`./${config.appDirectory}/**/*`],
-    });
-    let result = shell.grep("-l", 'from "', JSFiles);
-    expect(result.stdout.trim()).toBe("");
-    expect(result.stderr).toBeNull();
-    expect(result.code).toBe(0);
+    await runConvertToJavaScriptMigrationProgrammatically(projectDir);
 
-    expect(output).toContain("successfully migrated");
+    await checkMigrationRanSuccessfully(projectDir);
+
+    expect(output).not.toContain("✅ Your JavaScript looks good!");
+    expect(output).not.toContain("successfully migrated");
   });
 });