Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support JS conversion for app code without jscodeshift #5163

Merged
merged 5 commits into from
Jan 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 101 additions & 0 deletions decisions/0008-only-support-js-conversion-for-app-code.md
Original file line number Diff line number Diff line change
@@ -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
6 changes: 0 additions & 6 deletions packages/remix-dev/__tests__/cli-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
});
Expand Down
2 changes: 2 additions & 0 deletions packages/remix-dev/__tests__/codemod-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
2 changes: 0 additions & 2 deletions packages/remix-dev/__tests__/create-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
138 changes: 0 additions & 138 deletions packages/remix-dev/__tests__/migrations/convert-to-javascript-test.ts

This file was deleted.

43 changes: 43 additions & 0 deletions packages/remix-dev/__tests__/useJavascript-test.ts
Original file line number Diff line number Diff line change
@@ -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")');
});
6 changes: 3 additions & 3 deletions packages/remix-dev/cli/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
}
Expand Down
8 changes: 0 additions & 8 deletions packages/remix-dev/cli/migrate/flags.ts

This file was deleted.

52 changes: 0 additions & 52 deletions packages/remix-dev/cli/migrate/jscodeshift.ts

This file was deleted.

Loading