Skip to content

Commit

Permalink
refactor(remix-dev): rewrite jscodeshift-based migrations as `babel…
Browse files Browse the repository at this point in the history
…`-based codemods (#4572)

* refactor(dev): extract `useColor` usages into `safe` utility

* refactor(dev): remove outdated reference to compiler shims

* fix(dev): remove access to `convert-to-javascript` migration via CLI

conversion to javascript is not a "migration" in that it does not help the user to upgrade to a
newer version of Remix

* refactor(dev): rewrite `replace-remix-magic-exports` as a babel codemod

Compared to the previous jscodeshift-based migration:
- codemod no longer depends on a network connection
- babel's visitor API for traversing the AST is simpler
- not spinning up workers for applying code transforms

This ends up speeding up the codemod by ~10x and (hopefully 🤞) fixes
some of the issues we were seeing in CI on Windows (since we think
problems are mostly timeouts caused by slow tests or overhead for
workers).

* test(dev): restore fixture

this fixture was incorrectly included as part of dependency upgrades:
- #3917
- #3929

it should not have been updated since it is supposed to represent a
Remix 1.3.x codebase

* test(dev): add tests for generic codemods and specifically for `replace-remix-magic-imports`

* ci: run build before primary tests

some tests can make use of the built javascript artifacts
(e.g. `cli.js`) to run 8-10x faster than running with source
Typescript (e.g. `cli.ts`)

* test(dev): retry temp dir removal for windows ci

Windows sometimes throws `EBUSY: resource busy or locked, rmdir`
errors when attempting to removing the temporary directory.
Retrying a couple times seems to get it to succeed.
See https://github.com/jprichardson/node-fs-extra/issues?q=EBUSY%3A+resource+busy+or+locked%2C+rmdir

* Create long-colts-remain.md
  • Loading branch information
pcattori committed Nov 15, 2022
1 parent 95b7fd1 commit 70784af
Show file tree
Hide file tree
Showing 85 changed files with 1,970 additions and 1,978 deletions.
10 changes: 10 additions & 0 deletions .changeset/long-colts-remain.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
"remix": patch
"@remix-run/dev": patch
---

Replace migrations with codemods. Specifically, `npx @remix-run/dev migrate` is now `@remix-run/dev codemod`.

Under the hood, codemods are now written via Babel's Visitor API instead of jscodeshift.
Also `replace-remix-magic-imports` is now faster as it no longer depends on a network connection
and does not incur the overhead of spinning up workers for jscodeshift.
4 changes: 4 additions & 0 deletions .github/workflows/reusable-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ jobs:
- name: 📥 Install deps
run: yarn --frozen-lockfile

# It's faster to use the built `cli.js` in tests if its available and up-to-date
- name: 🏗 Build
run: yarn build

- name: 🧪 Run Primary Tests
run: "yarn test:primary"

Expand Down
9 changes: 3 additions & 6 deletions packages/remix-dev/__tests__/cli-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ describe("remix CLI", () => {
$ remix routes [projectDir]
$ remix watch [projectDir]
$ remix setup [remixPlatform]
$ remix migrate [-m migration] [projectDir]
$ remix codemod <codemod> [projectDir]
Options:
--help, -h Print this help message and exit
Expand All @@ -119,17 +119,14 @@ describe("remix CLI", () => {
--no-delete Skip deleting the \`remix.init\` script
\`routes\` Options:
--json Print the routes as JSON
\`migrate\` Options:
--debug Show debugging logs
\`codemod\` Options:
--dry Dry run (no changes are made to files)
--force Bypass Git safety checks and forcibly run migration
--migration, -m Name of the migration to run
--force Bypass Git safety checks
Values:
- projectDir The Remix project directory
- template The project template to use
- remixPlatform \`node\` or \`cloudflare\`
- migration One of the choices from https://github.com/remix-run/remix/blob/main/packages/remix-dev/cli/migrate/migrations/index.ts
Creating a new project:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
import NpmCliPackageJson from "@npmcli/package-json";
import fs from "fs";
import glob from "fast-glob";
import path from "path";
import shell from "shelljs";
import stripAnsi from "strip-ansi";

import { readConfig } from "../config";
import * as cli from "./utils/cli";
import * as eol from "./utils/eol";
import * as git from "./utils/git";
import withApp from "./utils/withApp";

let CODEMOD = "replace-remix-magic-imports";
let FIXTURE = path.join(__dirname, "fixtures/replace-remix-magic-imports");

it("replaces `remix` magic imports", async () => {
await withApp(FIXTURE, async (projectDir) => {
await git.initialCommit(projectDir);
let result = await cli.run(["codemod", CODEMOD, projectDir]);
let stderr = stripAnsi(result.stderr);
expect(result.exitCode).toBe(0);

let successes = [
`✔ Found codemod: ${CODEMOD}`,
"✔ No Remix server adapter detected",
"✔ Detected Remix server runtime: node",
"✔ Removed magic `remix` package from dependencies",
"✔ Removed `remix setup` from postinstall script",
"✔ Replaced magic `remix` imports | 24/24 files",
];
for (let success of successes) {
expect(stderr).toContain(success);
}

expect(result.stdout).toContain(
"👉 To update your lockfile, run `yarn install`"
);

let pkg = await NpmCliPackageJson.load(projectDir);
let packageJson = pkg.content;

// check that `remix` dependency was removed
expect(packageJson.dependencies).not.toContain("remix");
expect(packageJson.devDependencies).not.toContain("remix");

// check that Remix packages were standardized
expect(packageJson.dependencies).toEqual(
expect.objectContaining({
"@remix-run/node": "1.3.4",
"@remix-run/react": "1.3.4",
"@remix-run/serve": "1.3.4",
})
);
expect(packageJson.devDependencies).toEqual(
expect.objectContaining({
"@remix-run/dev": "1.3.4",
})
);

// check that postinstall was removed
expect(packageJson.scripts).not.toContain("postinstall");

// check that `from "remix"` magic imports were removed
let config = await readConfig(projectDir);
let files = await glob("**/*.{js,jsx,ts,tsx}", {
cwd: config.appDirectory,
absolute: true,
});
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 = eol.normalize(
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")
);
});
});
56 changes: 56 additions & 0 deletions packages/remix-dev/__tests__/codemod-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import fs from "fs";
import path from "path";

import * as cli from "./utils/cli";
import * as git from "./utils/git";
import withApp from "./utils/withApp";

let FIXTURE = path.join(__dirname, "fixtures/replace-remix-magic-imports");

it("checks that project is a clean git repository", async () => {
await withApp(FIXTURE, async (projectDir) => {
// ensure project is a git repository
let error1 = await cli.shouldError([
"codemod",
"some-codemod-name",
projectDir,
]);
expect(error1.exitCode).not.toBe(0);
expect(error1.stderr).toContain(`${projectDir} is not a git repository`);
expect(error1.stdout).toContain(
"To override this safety check, use the --force flag"
);

// initialize git repo in project
await git.initialCommit(projectDir);

// make some uncommitted changes
fs.appendFileSync(path.join(projectDir, "package.json"), "change");

// ensure project has no uncommitted changes
let error2 = await cli.shouldError([
"codemod",
"some-codemod-name",
projectDir,
]);
expect(error2.exitCode).not.toBe(0);
expect(error2.stderr).toContain(`${projectDir} has uncommitted changes`);
expect(error2.stdout).toContain(
"Stash or commit your changes before running codemods"
);
expect(error2.stdout).toContain(
"To override this safety check, use the --force flag"
);
});
});

it("checks that the specified codemod exists", async () => {
await withApp(FIXTURE, async (projectDir) => {
await git.initialCommit(projectDir);

let codemodName = "invalid-codemod-name";
let error = await cli.shouldError(["codemod", codemodName, projectDir]);
expect(error.exitCode).toBe(1);
expect(error.stderr).toContain(`Unrecognized codemod: ${codemodName}`);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@
"@remix-run/react": "1.3.4",
"@remix-run/serve": "1.3.4",
"marked": "^4.0.12",
"react": "^18.2.0",
"react-dom": "^18.2.0",
"react": "^17.0.2",
"react-dom": "^17.0.2",
"remix": "1.3.4",
"tiny-invariant": "^1.2.0"
},
Expand All @@ -35,12 +35,12 @@
"@remix-run/eslint-config": "1.3.4",
"@testing-library/cypress": "^8.0.2",
"@testing-library/jest-dom": "^5.16.2",
"@testing-library/react": "^13.3.0",
"@testing-library/react": "^12.1.4",
"@testing-library/user-event": "^13.5.0",
"@types/eslint": "^8.4.6",
"@types/marked": "^4.0.2",
"@types/react": "^18.0.15",
"@types/react-dom": "^18.0.6",
"@types/react": "^17.0.40",
"@types/react-dom": "^17.0.13",
"@vitejs/plugin-react": "^1.2.0",
"c8": "^7.11.0",
"cross-env": "^7.0.3",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
"skipLibCheck": true,

// Remix takes care of building everything in `remix build`.
"noEmit": true
"noEmit": true,
"allowJs": true,
"forceConsistentCasingInFileNames": true
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { spawnSync } from "child_process";
import { tmpdir } from "os";
import { join, resolve } from "path";
import { join } from "path";
import glob from "fast-glob";
import {
copySync,
Expand All @@ -13,8 +12,8 @@ import shell from "shelljs";
import stripAnsi from "strip-ansi";
import type { PackageJson, TsConfigJson } from "type-fest";

import { run } from "../../cli/run";
import { readConfig } from "../../config";
import { convertToJavaScript } from "../../cli/migrate/migrations/convert-to-javascript";

let output: string;
const ORIGINAL_IO = {
Expand Down Expand Up @@ -107,46 +106,11 @@ const makeApp = () => {
return projectDir;
};

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 when ran via CLI", async () => {
let projectDir = makeApp();

let output = runConvertToJavaScriptMigrationViaCLI(projectDir);

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();

await runConvertToJavaScriptMigrationProgrammatically(projectDir);
await convertToJavaScript(projectDir, { force: true });

await checkMigrationRanSuccessfully(projectDir);

Expand Down

0 comments on commit 70784af

Please sign in to comment.