Code and interface improvements after enabling some eslint recommended rules requiring type checking

.changeset/big-pens-search.md

@@ -0,0 +1,5 @@
+---
+"@remix-run/eslint-config": minor
+---
+
+Added `internal-type-checked.js` and `rules/type-checked.js` to `remix-eslint-config` to handle typescript-eslint rules that require type information and using the `@typescript-eslint/parser`.

.changeset/polite-garlics-invite.md

@@ -0,0 +1,5 @@
+---
+"@remix-run/server-runtime": minor
+---
+
+Added `this: void` to functions in the `SessionStorage` and `SessionIdStorageStrategy` interfaces so destructuring is correctly part of the contract.

.changeset/tough-roses-admire.md

@@ -0,0 +1,9 @@
+---
+"remix": patch
+"@remix-run/dev": patch
+"@remix-run/react": patch
+"@remix-run/serve": patch
+"@remix-run/testing": patch
+---
+
+Added `void` operator to Promises that are not being awaited. Updated `@types/react` and `@types/react-dom` to most recent release. Used `await` on finally retry in test.

.github/workflows/lint.yml

@@ -36,6 +36,8 @@ jobs:
 
       - name: 🔬 Lint
         run: yarn lint
+        env:
+          NODE_OPTIONS: "--max-old-space-size=6144"
 
       - name: 🦕 Install Deno
         uses: denoland/setup-deno@v1

.vscode/settings.json

@@ -1,5 +1,7 @@
 {
   "typescript.tsdk": "node_modules/typescript/lib",
   "deno.enablePaths": ["./packages/remix-deno/"],
-  "deno.importMap": "./.vscode/deno_resolve_npm_imports.json"
+  "deno.importMap": "./.vscode/deno_resolve_npm_imports.json",
+  "eslint.packageManager": "yarn",
+  "eslint.execArgv": ["--max-old-space-size=6144"]
 }

contributors.yml

@@ -371,6 +371,7 @@
 - nareshbhatia
 - navid-kalaei
 - nexxeln
+- ngbrown
 - ni554n
 - nicholaschiang
 - nicksrandall

package.json

@@ -78,8 +78,8 @@
     "@types/jest": "^27.4.1",
     "@types/jsonfile": "^6.1.0",
     "@types/lodash": "^4.14.182",
-    "@types/react": "^18.0.15",
-    "@types/react-dom": "^18.0.6",
+    "@types/react": "^18.2.5",
+    "@types/react-dom": "^18.2.4",
     "@types/react-test-renderer": "^18.0.0",
     "@types/retry": "^0.12.0",
     "@types/semver": "^7.3.4",

packages/remix-dev/__tests__/utils/withApp.ts

@@ -36,6 +36,6 @@ export default async <Result>(
     // 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
-    retry(async () => await fse.remove(TEMP_DIR), 3, 200);
+    await retry(async () => await fse.remove(TEMP_DIR), 3, 200);
   }
 };

packages/remix-dev/cli/commands.ts

@@ -208,7 +208,7 @@ export async function watch(
       ? remixRootOrConfig
       : await readConfig(remixRootOrConfig);
 
-  devServer.liveReload(config);
+  void devServer.liveReload(config);
   return await new Promise(() => {});
 }
 

packages/remix-dev/compiler/compiler.ts

@@ -53,7 +53,7 @@ export let create = async (ctx: Context): Promise<Compiler> => {
       if (error === undefined) {
         error = thrown;
       }
-      cancel();
+      void cancel();
       return err(thrown);
     };
 

packages/remix-dev/compiler/watch.ts

@@ -58,7 +58,7 @@ export async function watch(
 
   let restart = debounce(async () => {
     let start = Date.now();
-    compiler.dispose();
+    void compiler.dispose();
 
     try {
       ctx.config = await reloadConfig(ctx.config.rootDirectory);
@@ -123,6 +123,6 @@ export async function watch(
 
   return async () => {
     await watcher.close().catch(() => undefined);
-    compiler.dispose();
+    void compiler.dispose();
   };
 }

packages/remix-dev/devServer/serve.ts

@@ -84,7 +84,7 @@ export async function serve(config: RemixConfig, portPreference?: number) {
       ? app.listen(port, process.env.HOST, onListen)
       : app.listen(port, onListen);
   } catch {
-    dispose();
+    void dispose();
     server?.close();
   }
 }

packages/remix-eslint-config/internal-type-checked.js

@@ -0,0 +1,34 @@
+const typeCheckedRules = require("./rules/type-checked");
+
+/**
+ * @see https://github.com/eslint/eslint/issues/3458
+ * @see https://www.npmjs.com/package/@rushstack/eslint-patch
+ */
+require("@rushstack/eslint-patch/modern-module-resolution");
+
+module.exports = {
+  plugins: [],
+  overrides: [
+    {
+      files: [
+        "packages/**/*.ts?(x)",
+        "templates/**/*.ts?(x)",
+        "scripts/playground/**/*.ts?(x)",
+      ],
+      excludedFiles: ["**/*.md/**"],
+      rules: {
+        ...typeCheckedRules,
+      },
+      parser: "@typescript-eslint/parser",
+      parserOptions: {
+        tsconfigRootDir: __dirname,
+        project: [
+          "../*/tsconfig.json",
+          "../../templates/*/tsconfig.json",
+          "../../scripts/playground/template/tsconfig.json",
+          "../../tsconfig.eslint.json",
+        ],
+      },
+    },
+  ],
+};

packages/remix-eslint-config/internal.js

@@ -19,6 +19,7 @@ module.exports = {
   extends: [
     require.resolve("./index.js"),
     require.resolve("./jest-testing-library.js"),
+    require.resolve("./internal-type-checked.js"),
   ],
   env: {
     node: true,
@@ -85,5 +86,13 @@ module.exports = {
         "jest/globals": false,
       },
     },
+    {
+      files: ["**/__tests__/**/*", "**/*.{spec,test}.*"],
+      rules: {
+        // turn the original rule off *only* for test files
+        "@typescript-eslint/unbound-method": OFF,
+        "jest/unbound-method": WARN,
+      },
+    },
   ],
 };

packages/remix-eslint-config/rules/type-checked.js

@@ -0,0 +1,32 @@
+const OFF = 0;
+const WARN = 1;
+const ERROR = 2;
+
+module.exports = {
+  // Additional recommended rules that require type information, to prepare for when end applications enable
+  // "plugin:@typescript-eslint/recommended-requiring-type-checking".
+  // Include individual rules from https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/src/configs/recommended-requiring-type-checking.ts
+  // Requires more memory: NODE_OPTIONS="--max-old-space-size=6144"
+
+  "@typescript-eslint/await-thenable": ERROR,
+  "@typescript-eslint/no-floating-promises": [
+    ERROR,
+    { ignoreVoid: true, ignoreIIFE: true },
+  ],
+  "@typescript-eslint/no-for-in-array": ERROR,
+  "@typescript-eslint/no-implied-eval": ERROR,
+  "@typescript-eslint/no-misused-promises": [
+    ERROR,
+    { checksVoidReturn: { arguments: false, properties: false } },
+  ],
+  "@typescript-eslint/no-unnecessary-type-assertion": OFF,
+  "@typescript-eslint/no-unsafe-argument": OFF,
+  "@typescript-eslint/no-unsafe-assignment": OFF,
+  "@typescript-eslint/no-unsafe-call": OFF,
+  "@typescript-eslint/no-unsafe-member-access": OFF,
+  "@typescript-eslint/no-unsafe-return": OFF,
+  "@typescript-eslint/require-await": OFF,
+  "@typescript-eslint/restrict-plus-operands": OFF,
+  "@typescript-eslint/restrict-template-expressions": OFF,
+  "@typescript-eslint/unbound-method": WARN,
+};

packages/remix-node/__tests__/fileUploadHandler-test.ts

@@ -33,73 +33,73 @@ describe("NodeOnDiskFile", () => {
   });
 
   it("can slice file and change type", async () => {
-    let sliced = await file.slice(1, 5, "text/rofl");
+    let sliced = file.slice(1, 5, "text/rofl");
     expect(sliced.type).toBe("text/rofl");
     expect(await sliced.text()).toBe(contents.slice(1, 5));
   });
 
   it("can slice file and get text", async () => {
-    let sliced = await file.slice(1, 5);
+    let sliced = file.slice(1, 5);
     expect(await sliced.text()).toBe(contents.slice(1, 5));
   });
 
   it("can slice file twice and get text", async () => {
-    let sliced = (await file.slice(1, 5)).slice(1, 2);
+    let sliced = (file.slice(1, 5)).slice(1, 2);
     expect(await sliced.text()).toBe(contents.slice(1, 5).slice(1, 2));
   });
 
   it("can sice file and get an arrayBuffer", async () => {
-    let sliced = await file.slice(1, 5);
+    let sliced = file.slice(1, 5);
     let slicedRes = contents.slice(1, 5);
     let buffer = await sliced.arrayBuffer();
     expect(buffer.byteLength).toBe(slicedRes.length);
     expect(buffer).toEqual(Buffer.from(slicedRes));
   });
 
   it("can slice file and use stream", async () => {
-    let sliced = await file.slice(1, 5);
+    let sliced = file.slice(1, 5);
     let slicedRes = contents.slice(1, 5);
     expect(sliced.size).toBe(slicedRes.length);
     expect(await sliced.text()).toBe(slicedRes);
   });
 
   it("can slice file with negative start and no end", async () => {
-    let sliced = await file.slice(-2);
+    let sliced = file.slice(-2);
     let slicedRes = contents.slice(-2);
     expect(sliced.size).toBe(slicedRes.length);
     expect(await sliced.text()).toBe(slicedRes);
   });
 
   it("can slice file with negative start and negative end", async () => {
-    let sliced = await file.slice(-3, -1);
+    let sliced = file.slice(-3, -1);
     let slicedRes = contents.slice(-3, -1);
     expect(sliced.size).toBe(slicedRes.length);
     expect(await sliced.text()).toBe(slicedRes);
   });
 
   it("can slice file with negative start and negative end twice", async () => {
-    let sliced = await file.slice(-3, -1).slice(1, -1);
+    let sliced = file.slice(-3, -1).slice(1, -1);
     let slicedRes = contents.slice(-3, -1).slice(1, -1);
     expect(sliced.size).toBe(slicedRes.length);
     expect(await sliced.text()).toBe(slicedRes);
   });
 
   it("can slice file with start and negative end", async () => {
-    let sliced = await file.slice(1, -2);
+    let sliced = file.slice(1, -2);
     let slicedRes = contents.slice(1, -2);
     expect(sliced.size).toBe(slicedRes.length);
     expect(await sliced.text()).toBe(slicedRes);
   });
 
   it("can slice file with negaive start and end", async () => {
-    let sliced = await file.slice(-3, 1);
+    let sliced = file.slice(-3, 1);
     let slicedRes = contents.slice(-3, 1);
     expect(sliced.size).toBe(slicedRes.length);
     expect(await sliced.text()).toBe(slicedRes);
   });
 
   it("can slice oob", async () => {
-    let sliced = await file.slice(0, 10000);
+    let sliced = file.slice(0, 10000);
     let slicedRes = contents.slice(0, 10000);
     expect(sliced.size).toBe(slicedRes.length);
     expect(await sliced.text()).toBe(slicedRes);

packages/remix-node/stream.ts

@@ -107,6 +107,9 @@ class StreamPump {
       new Stream.Readable().readableHighWaterMark;
     this.accumalatedSize = 0;
     this.stream = stream;
+
+    // see [unbound-method] support binding methods inside constructor
+    // https://github.com/typescript-eslint/typescript-eslint/issues/636
     this.enqueue = this.enqueue.bind(this);
     this.error = this.error.bind(this);
     this.close = this.close.bind(this);

packages/remix-react/components.tsx

@@ -496,7 +496,7 @@ function usePrefetchedStylesheets(matches: AgnosticDataRouteMatch[]) {
   React.useEffect(() => {
     let interrupted: boolean = false;
 
-    getStylesheetPrefetchLinks(matches, manifest, routeModules).then(
+    void getStylesheetPrefetchLinks(matches, manifest, routeModules).then(
       (links) => {
         if (!interrupted) setStyleLinks(links);
       }

packages/remix-server-runtime/package.json

@@ -25,7 +25,7 @@
   "devDependencies": {
     "@remix-run/web-file": "^3.0.2",
     "@types/cookie": "^0.4.0",
-    "@types/react": "^18.0.15",
+    "@types/react": "^18.2.5",
     "@types/set-cookie-parser": "^2.4.1"
   },
   "engines": {

packages/remix-server-runtime/sessions.ts

@@ -175,6 +175,7 @@ export interface SessionStorage<Data = SessionData, FlashData = Data> {
    * return a new Session with no data.
    */
   getSession(
+    this: void,
     cookieHeader?: string | null,
     options?: CookieParseOptions
   ): Promise<Session<Data, FlashData>>;
@@ -184,6 +185,7 @@ export interface SessionStorage<Data = SessionData, FlashData = Data> {
    * used in the HTTP response.
    */
   commitSession(
+    this: void,
     session: Session<Data, FlashData>,
     options?: CookieSerializeOptions
   ): Promise<string>;
@@ -193,6 +195,7 @@ export interface SessionStorage<Data = SessionData, FlashData = Data> {
    * header to be used in the HTTP response.
    */
   destroySession(
+    this: void,
     session: Session<Data, FlashData>,
     options?: CookieSerializeOptions
   ): Promise<string>;
@@ -221,19 +224,21 @@ export interface SessionIdStorageStrategy<
    * Creates a new record with the given data and returns the session id.
    */
   createData: (
+    this: void,
     data: FlashSessionData<Data, FlashData>,
     expires?: Date
   ) => Promise<string>;
 
   /**
    * Returns data for a given session id, or `null` if there isn't any.
    */
-  readData: (id: string) => Promise<FlashSessionData<Data, FlashData> | null>;
+  readData: (this: void, id: string) => Promise<FlashSessionData<Data, FlashData> | null>;
 
   /**
    * Updates data for the given session id.
    */
   updateData: (
+    this: void,
     id: string,
     data: FlashSessionData<Data, FlashData>,
     expires?: Date
@@ -242,7 +247,7 @@ export interface SessionIdStorageStrategy<
   /**
    * Deletes data for a given session id from the data store.
    */
-  deleteData: (id: string) => Promise<void>;
+  deleteData: (this: void, id: string) => Promise<void>;
 }
 
 export type CreateSessionStorageFunction = <

packages/remix-testing/package.json

@@ -23,8 +23,8 @@
   },
   "devDependencies": {
     "@types/node": "^18.11.9",
-    "@types/react": "^18.0.24",
-    "@types/react-dom": "^18.0.8",
+    "@types/react": "^18.2.5",
+    "@types/react-dom": "^18.2.4",
     "react": "^18.2.0",
     "react-dom": "^18.2.0",
     "typescript": "^5.0.4"

tsconfig.eslint.json

@@ -0,0 +1,17 @@
+{
+  "extends": "./tsconfig.json",
+  "compilerOptions": {
+    "noEmit": true
+  },
+  "include": [
+    // whatever paths need a fallback tsconfig.json that you intend to lint
+    "packages"
+  ],
+  "lib": ["ES2019", "DOM", "DOM.Iterable"],
+  "composite": true,
+  "moduleResolution": "node",
+  "resolveJsonModule": true,
+  "allowSyntheticDefaultImports": true,
+  "strict": true,
+  "skipLibCheck": true,
+}