Support JS conversion for app code without jscodeshift (remix-run#5163

)

* docs(decisions): only support js conversion for app code

* refactor(dev): remove jscodeshift-based migration

* refactor(dev): only support ts->js conversion for app code

* test(dev): update ts->js conversion tests

* test(dev): bump jest timeout for codemod test

if there's an up-to-date build, this test is fast. but if not, it can take ~15-18s locally on my M1
MBP 16

decisions/0008-only-support-js-conversion-for-app-code.md

@@ -0,0 +1,101 @@
+# Only support JS conversion for app code
+
+Date: 2023-01-20
+
+Status: accepted
+
+## Context
+
+Remix defaults to Typescript, but some users prefer to use Javascript.
+When creating a new Remix project via `npx create-remix` today, the CLI asks the user if they'd
+prefer to use Typescript or Javascript.
+
+```sh
+❯ npx create-remix@latest
+? Where would you like to create your app? ./my-remix-app
+? What type of app do you want to create? Just the basics
+? Where do you want to deploy? Choose Remix App Server if you're unsure; it's easy to change deployment targets. Remix App Server
+? TypeScript or JavaScript? (Use arrow keys)
+❯ TypeScript
+  JavaScript
+```
+
+Originally, this was implemented by having separate variants of a template for TS and JS.
+This worked, but duplicated huge portions of templates making those variants hard to maintain.
+To fix this, the decision was made to maintain _only_ the TS variant of each template.
+To provide JS-only Remix projects, the Remix CLI would copy the TS template and then dynamically
+convert all Typescript-related code to their Javascript equivalents.
+
+While converting the code in the Remix app directory (e.g. `app/`) is reliable, conversion of
+TS-related code outside of the `app/` directory are tricky and error-prone.
+
+### 1 Stale references to `.ts` files
+
+For example, both the [Indie][indie-stack] and [Blues][blues-stack] stacks have broken scripts when selecting `Javascript`,
+because they still reference `server.ts` and `seed.ts`.
+They also still reference TS-only tools like `ts-node` in their scripts.
+
+### 2.a MJS
+
+When transpiling code outside of the `app/` directory from TS to JS, the easiest thing to do is
+to convert to ESM-style `.mjs` since Remix app code already uses ESM.
+
+ESM-style JS requires one of the following:
+
+- a) Set `"type": "module"` in `package.json`
+- b) Use `.mjs` extension
+
+(a) immediately breaks Remix's builds since the settings in `package.json` apply to app code, not just code and
+scripts outside of the app directory.
+
+(b) seems more promising, but `.mjs` files require file extensions for import specifiers.
+Reliably changing relative imports without extensions to have the proper extensions is
+untractable since not all imports will be for `.js` files.
+
+```js
+// ./script.mjs (converted from ./script.js)
+import myHelper from "./my-helper";
+
+// Should this be converted to `./my-helper.mjs`?
+// Probably, but can we be sure?
+
+myHelper();
+```
+
+Maybe there's a way to do this, but the complexity cost seems high.
+
+### 2.b CJS
+
+If we don't use the `.mjs` extension, Node will default to treating scripts and code outside of the app directory
+as CJS.
+Since CJS doesn't support ESM-style `import`/`export`, we'd then need to convert all `import`s and `export`s to their
+equivalent `require`/`module.exports`.
+
+Important to remember that the converted code is meant to be read and edited by other developers, so its not acceptable
+to produce a bunch of boilerplate adapters for the imports and exports as would be typical in build output.
+
+Converting the imports and exports may be doable, but again, carries high complexity cost.
+
+## Decision
+
+Only support JS conversion for app code, not for scripts or code outside of the Remix app directory.
+
+## Consequences
+
+Users will have three options:
+
+1. Use a Typescript template
+2. Use a Typescript template with app directory converted to JS
+3. Use a dedicated Javascript template
+
+### Converting remaining TS to JS
+
+If you don't like the remaining TS from option (2) and cannot find a suitable template for option (3),
+you can still remove any remaining Typescript code manually:
+
+- Remove `tsconfig.json` or replace it with the equivalent `jsconfig.json`
+- Replace TS-only tools with their JS counterparts (e.g. `ts-node` -> `node`)
+- Change any remaining `.ts` files to `.mjs` and update imports and other references (like `package.json` scripts) to refer to the new filename
+
+[indie-stack]: https://github.com/remix-run/indie-stack
+[blues-stack]: https://github.com/remix-run/blues-stack

packages/remix-dev/__tests__/cli-test.ts

@@ -248,12 +248,6 @@ describe("remix CLI", () => {
       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();
     });
   });
 });

packages/remix-dev/__tests__/codemod-test.ts

@@ -5,6 +5,8 @@ import * as cli from "./utils/cli";
 import * as git from "./utils/git";
 import withApp from "./utils/withApp";
 
+jest.setTimeout(1000 * 20);
+
 let FIXTURE = path.join(__dirname, "fixtures/replace-remix-magic-imports");
 
 it("checks that project is a clean git repository", async () => {

packages/remix-dev/__tests__/create-test.ts

@@ -353,8 +353,6 @@ describe("the create command", () => {
     );
     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();
   });
 
   it("works for a file path to a directory on disk", async () => {

packages/remix-dev/__tests__/useJavascript-test.ts

@@ -0,0 +1,43 @@
+import { tmpdir } from "os";
+import path from "path";
+import glob from "fast-glob";
+import fs from "fs-extra";
+
+import { readConfig } from "../config";
+import * as useJavascript from "../cli/useJavascript";
+
+const FIXTURE = path.join(__dirname, "fixtures", "indie-stack");
+const TEMP_DIR = path.join(
+  fs.realpathSync(tmpdir()),
+  `remix-tests-${Math.random().toString(32).slice(2)}`
+);
+
+let makeApp = () => {
+  let projectDir = path.join(TEMP_DIR, "convert-to-javascript");
+  fs.copySync(FIXTURE, projectDir);
+  return projectDir;
+};
+
+it("useJavascript converts app code from TS to JS", async () => {
+  let projectDir = makeApp();
+  await useJavascript.convert(projectDir);
+
+  let config = await readConfig(projectDir);
+
+  // no remix.env.d.ts
+  let remixEnvD = path.join(config.appDirectory, "remix.env.d.ts");
+  expect(fs.existsSync(remixEnvD)).toBeFalsy();
+
+  // no TS files within app directory
+  let TSFiles = glob.sync("**/*.{d.ts,ts,tsx}", {
+    cwd: config.appDirectory, // todo: normalize this?
+  });
+  expect(TSFiles).toHaveLength(0);
+
+  // ensure ESM imports in app directory
+  let rootRoute = await fs.readFile(
+    path.join(projectDir, "app", "root.jsx"),
+    "utf-8"
+  );
+  expect(rootRoute).not.toContain('require("@remix-run/react")');
+});

packages/remix-dev/cli/create.ts

@@ -16,7 +16,7 @@ import * as colors from "../colors";
 import invariant from "../invariant";
 import packageJson from "../package.json";
 import { getPreferredPackageManager } from "./getPreferredPackageManager";
-import { convertToJavaScript } from "./migrate/migrations/convert-to-javascript";
+import * as useJavascript from "./useJavascript";
 
 const remixDevPackageVersion = packageJson.version;
 const defaultAgent = new ProxyAgent();
@@ -201,8 +201,8 @@ export async function createApp({
     !useTypeScript &&
     fse.existsSync(path.join(projectDir, "tsconfig.json"))
   ) {
-    let spinner = ora("Migrating template to JavaScript…").start();
-    await convertToJavaScript(projectDir, { interactive: false });
+    let spinner = ora("Converting template to JavaScript…").start();
+    await useJavascript.convert(projectDir);
     spinner.stop();
     spinner.clear();
   }