Repository: remix-run/remix
Pull request #2786 (2022-04-13 20:42:59)

fix(remix-dev): apply suggested and mandatory tsconfig when values are missing
Add some tsconfig (and jsconfig) compilerOptions, some of these are required by esbuild, others are our options from our existing templates. Being that our tsconfig resolver walks the fs, i have the baseUrl resolving from that


<img width="570" alt="image" src="https://user-images.githubusercontent.com/11698668/164088283-3f45fcf0-625b-4519-86c2-8368f712f4dc.png">


Signed-off-by: Logan McAnsh <logan@mcan.sh>

jacob-ebey: Could we potentially set it to their app dir if it's not set? Or is it better to just alert about the misconfiguration? 
mcansh: > Could we potentially set it to their app dir if it's not set? Or is it better to just alert about the misconfiguration?

for sure, I was going back on forth on it
mcansh: gonna implement this tomorrow, do we want to suggest/enforce any other compilerOptions? maybe the config from our templates?
mcansh: main goal for the PR is done, but going to also check for the required tsconfig settings for esbuild
mcansh: 👌 
<img width="570" alt="image" src="https://user-images.githubusercontent.com/11698668/164088283-3f45fcf0-625b-4519-86c2-8368f712f4dc.png">

mcansh: only downside, is the build is now "slow" as it has check for these settings, I'll look at just doing the check "remix dev"
mcansh: actually, idk why that shows it being so slow, i'm seeing it a consistent ~520ms

Patch:
diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml
index 3ef129d99..93fdd7510 100644
--- a/.github/workflows/nightly.yml
+++ b/.github/workflows/nightly.yml
@@ -40,9 +40,6 @@ jobs:
         # because we have a yarn.lock and caches efficiently.
         uses: bahmutov/npm-install@v1
 
-      - name: 🏗 Build
-        run: yarn build
-
       - name: ⤴️ Update Version
         id: version
         run: |
@@ -67,7 +64,11 @@ jobs:
 
           yarn run version ${NEXT_VERSION} --skip-prompt
 
-          git push origin --tags
+      - name: 🏗 Build
+        run: yarn build
+
+      - name: 🏷 Push Tag
+        run: git push origin --tags
 
       - name: 🔐 Setup npm auth
         run: |
@@ -90,7 +91,7 @@ jobs:
 
   comment:
     needs: [nightly]
-    name: Comment on Issues and PRs
+    name: 🛴 Kick off comment and deployment test workflows
     if: github.repository == 'remix-run/remix'
     runs-on: ubuntu-latest
     steps:
@@ -102,6 +103,7 @@ jobs:
           VERSION: refs/tags/v${{ needs.nightly.outputs.NEXT_VERSION }}
         run: |
           curl \
+            -H "Accept: application/json" \
             -H "Authorization: token ${GITHUB_TOKEN}" \
             --request POST \
             --data '{ "event_type": "${EVENT_TYPE}", "client_payload": { "ref": "${VERSION}" } }' \
diff --git a/.github/workflows/release-comments.yml b/.github/workflows/release-comments.yml
index a35fdfc44..d918d4ccc 100644
--- a/.github/workflows/release-comments.yml
+++ b/.github/workflows/release-comments.yml
@@ -29,4 +29,4 @@ jobs:
         env:
           GITHUB_REPOSITORY: ${{ github.repository }}
           GITHUB_TOKEN: ${{ github.token }}
-          GITHUB_REF: ${{ github.event.client_payload.ref }}
+          VERSION: ${{ github.event.client_payload.ref }}
diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml
index b2de48e6e..9219b9d66 100644
--- a/.github/workflows/release.yml
+++ b/.github/workflows/release.yml
@@ -46,11 +46,11 @@ jobs:
 
   comment:
     needs: [manual]
-    name: Comment on Issues and PRs
+    name: 🛴 Kick off comment and deployment test workflows
     if: github.repository == 'remix-run/remix'
     runs-on: ubuntu-latest
     steps:
-      - name: 🛴 Kick of comment and deployment test workflows
+      - name: 🛴 Kick off comment and deployment test workflows
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
           EVENT_TYPE: release
@@ -58,6 +58,7 @@ jobs:
           VERSION: ${{ github.ref }}
         run: |
           curl \
+            -H "Accept: application/json" \
             -H "Authorization: token ${GITHUB_TOKEN}" \
             --request POST \
             --data '{ "event_type": "${EVENT_TYPE}", "client_payload": { "ref": "${VERSION}" } }' \
diff --git a/packages/remix-dev/cli/commands.ts b/packages/remix-dev/cli/commands.ts
index ea8b54e26..cd7f601f8 100644
--- a/packages/remix-dev/cli/commands.ts
+++ b/packages/remix-dev/cli/commands.ts
@@ -144,6 +144,9 @@ export async function build(
 
   let start = Date.now();
   let config = await readConfig(remixRoot);
+
+  cleanBuildDirectories(config);
+
   await compiler.build(config, {
     mode: mode,
     sourcemap,
@@ -193,6 +196,8 @@ export async function watch(
     broadcast({ type: "LOG", message });
   }
 
+  cleanBuildDirectories(config);
+
   let closeWatcher = await compiler.watch(config, {
     mode,
     onInitialBuild,
@@ -306,3 +311,12 @@ function purgeAppRequireCache(buildPath: string) {
     }
   }
 }
+
+function cleanBuildDirectories(config: RemixConfig) {
+  try {
+    fse.emptyDirSync(config.assetsBuildDirectory);
+    fse.emptyDirSync(path.dirname(config.serverBuildPath));
+  } catch (err) {
+    console.error("Could not clean build directories");
+  }
+}
diff --git a/packages/remix-dev/colors.ts b/packages/remix-dev/colors.ts
index 18916317e..73875ed47 100644
--- a/packages/remix-dev/colors.ts
+++ b/packages/remix-dev/colors.ts
@@ -21,3 +21,4 @@ export const logoRed = useColor ? chalk.redBright : K;
 
 export const gray = useColor ? chalk.gray : K;
 export const blue = useColor ? chalk.blue : K;
+export const bold = useColor ? chalk.bold : K;
diff --git a/packages/remix-dev/compiler/utils/tsconfig/index.ts b/packages/remix-dev/compiler/utils/tsconfig/index.ts
index 98ff267ba..c7d4ffd62 100644
--- a/packages/remix-dev/compiler/utils/tsconfig/index.ts
+++ b/packages/remix-dev/compiler/utils/tsconfig/index.ts
@@ -1,20 +1,26 @@
 import tsConfigPaths from "tsconfig-paths";
 
 import { loadTsConfig } from "./configLoader";
+import { writeConfigDefaults } from "./write-config-defaults";
 export { loadTsConfig } from "./configLoader";
 
 export function createMatchPath() {
   let configLoaderResult = loadTsConfig();
   if (configLoaderResult.resultType === "failed") {
+    if (configLoaderResult.message === "Missing baseUrl in compilerOptions") {
+      throw new Error(
+        `🚨 Oops! No baseUrl found, please set compilerOptions.baseUrl in your tsconfig or jsconfig`
+      );
+    }
     return undefined;
   }
 
-  let matchPath = tsConfigPaths.createMatchPath(
+  writeConfigDefaults(configLoaderResult.configFileAbsolutePath);
+
+  return tsConfigPaths.createMatchPath(
     configLoaderResult.absoluteBaseUrl,
     configLoaderResult.paths,
     configLoaderResult.mainFields,
     configLoaderResult.addMatchAll
   );
-
-  return matchPath;
 }
diff --git a/packages/remix-dev/compiler/utils/tsconfig/write-config-defaults.ts b/packages/remix-dev/compiler/utils/tsconfig/write-config-defaults.ts
new file mode 100644
index 000000000..550442e91
--- /dev/null
+++ b/packages/remix-dev/compiler/utils/tsconfig/write-config-defaults.ts
@@ -0,0 +1,117 @@
+import * as path from "path";
+import fse from "fs-extra";
+import JSON5 from "json5";
+import type { TsConfigJson } from "type-fest";
+import prettier from "prettier";
+
+import * as colors from "../../../colors";
+
+// These are suggested values and will be set when not present in the
+// tsconfig.json
+let suggestedCompilerOptions: TsConfigJson.CompilerOptions = {
+  forceConsistentCasingInFileNames: true,
+  target: "es2019",
+  lib: ["DOM", "DOM.Iterable", "ES2019"] as TsConfigJson.CompilerOptions.Lib[],
+  allowJs: true,
+  strict: true,
+  paths: {
+    "~/*": ["./app/*"],
+  },
+};
+
+// These values are required and cannot be changed by the user
+// Keep this in sync with esbuild
+let requiredCompilerOptions: TsConfigJson.CompilerOptions = {
+  esModuleInterop: true,
+  isolatedModules: true,
+  jsx: "react-jsx",
+  moduleResolution: "node",
+  resolveJsonModule: true,
+  noEmit: true,
+};
+
+// taken from https://github.com/sindresorhus/ts-extras/blob/781044f0412ec4a4224a1b9abce5ff0eacee3e72/source/object-keys.ts
+type ObjectKeys<T extends object> = `${Exclude<keyof T, symbol>}`;
+function objectKeys<Type extends object>(value: Type): Array<ObjectKeys<Type>> {
+  return Object.keys(value) as Array<ObjectKeys<Type>>;
+}
+
+export function writeConfigDefaults(configPath: string) {
+  let configContents = fse.readFileSync(configPath, "utf-8");
+  let config = JSON5.parse(configContents);
+  let configType = path.basename(configPath);
+  if (!config.compilerOptions) {
+    config.compilerOptions = {};
+  }
+  let suggestedChanges = [];
+  let requiredChanges = [];
+  if (!("include" in config)) {
+    config.include = ["remix.env.d.ts", "**/*.ts", "**/*.tsx"];
+    suggestedChanges.push(
+      colors.blue("include") +
+        " was set to " +
+        colors.bold(`['remix.env.d.ts', '**/*.ts', '**/*.tsx']`)
+    );
+  }
+  // TODO: check for user's typescript version and only add baseUrl if < 4.1
+  if (!("baseUrl" in config.compilerOptions)) {
+    let baseUrl = path.relative(process.cwd(), path.dirname(configPath)) || ".";
+    config.compilerOptions.baseUrl = baseUrl;
+    requiredChanges.push(
+      colors.blue("compilerOptions.baseUrl") +
+        " was set to " +
+        colors.bold(`'${baseUrl}'`)
+    );
+  }
+  for (let key of objectKeys(suggestedCompilerOptions)) {
+    if (!(key in config.compilerOptions)) {
+      config.compilerOptions[key] = suggestedCompilerOptions[key] as any;
+      suggestedChanges.push(
+        colors.blue("compilerOptions." + key) +
+          " was set to " +
+          colors.bold(`'${suggestedCompilerOptions[key]}'`)
+      );
+    }
+  }
+  for (let key of objectKeys(requiredCompilerOptions)) {
+    if (config.compilerOptions[key] !== requiredCompilerOptions[key]) {
+      config.compilerOptions[key] = requiredCompilerOptions[key] as any;
+      requiredChanges.push(
+        colors.blue("compilerOptions." + key) +
+          " was set to " +
+          colors.bold(`'${requiredCompilerOptions[key]}'`)
+      );
+    }
+  }
+  if (suggestedChanges.length > 0 || requiredChanges.length > 0) {
+    fse.writeFileSync(
+      configPath,
+      prettier.format(JSON.stringify(config, null, 2), {
+        parser: "json",
+      })
+    );
+  }
+  if (suggestedChanges.length > 0) {
+    console.log(
+      `The following suggested values were added to your ${colors.blue(
+        `"${configType}"`
+      )}. These values ${colors.bold(
+        "can be changed"
+      )} to fit your project's needs:\n`
+    );
+
+    suggestedChanges.forEach((change) => console.log(`\t- ${change}`));
+    console.log("");
+  }
+
+  if (requiredChanges.length > 0) {
+    console.log(
+      `The following ${colors.bold(
+        "mandatory changes"
+      )} were made to your ${colors.blue(configType)}:\n`
+    );
+
+    requiredChanges.forEach((change) => console.log(`\t- ${change}`));
+    console.log("");
+  }
+}
diff --git a/packages/remix-dev/config.ts b/packages/remix-dev/config.ts
index 3214b27bc..8f0cc2b91 100644
--- a/packages/remix-dev/config.ts
+++ b/packages/remix-dev/config.ts
@@ -353,7 +353,14 @@ export async function readConfig(
       path.join("public", "build")
   );
 
-  let devServerPort = await getPort({ port: appConfig.devServerPort || 8002 });
+  if (typeof appConfig.devServerPort !== "number") {
+    appConfig.devServerPort = Number(
+      process.env.REMIX_DEV_SERVER_WS_PORT || 8002
+    );
+  }
+  let devServerPort = await getPort({ port: appConfig.devServerPort });
+  // set env variable so un-bundled servers can use it
+  process.env.REMIX_DEV_SERVER_WS_PORT = `${devServerPort}`;
   let devServerBroadcastDelay = appConfig.devServerBroadcastDelay || 0;
 
   let defaultPublicPath = "/build/";
diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx
index b194d7275..a22655260 100644
--- a/packages/remix-react/components.tsx
+++ b/packages/remix-react/components.tsx
@@ -1392,9 +1392,10 @@ export function useFetchers(): Fetcher[] {
   return [...fetchers.values()];
 }
 
-let liveReloadMounted = false;
 // Dead Code Elimination magic for production builds.
 // This way devs don't have to worry about doing the NODE_ENV check themselves.
+// If running an un-bundled server outside of `remix dev` you will still need
+// to set the REMIX_DEV_SERVER_WS_PORT manually.
 export const LiveReload =
   process.env.NODE_ENV !== "development"
     ? () => null
@@ -1408,36 +1409,40 @@ export const LiveReload =
          */
         nonce?: string;
       }) {
-        let setupLiveReload = (port: number) => {
-          let protocol = location.protocol === "https:" ? "wss:" : "ws:";
-          let host = location.hostname;
-          let socketPath = `${protocol}//${host}:${port}/socket`;
-
-          let ws = new WebSocket(socketPath);
-          ws.onmessage = (message) => {
-            let event = JSON.parse(message.data);
-            if (event.type === "LOG") {
-              console.log(event.message);
-            }
-            if (event.type === "RELOAD") {
-              console.log("💿 Reloading window ...");
-              window.location.reload();
-            }
-          };
-          ws.onerror = (error) => {
-            console.log("Remix dev asset server web socket error:");
-            console.error(error);
-          };
-        };
-
-        React.useEffect(() => {
-          if (!liveReloadMounted) {
-            setupLiveReload(port);
-            liveReloadMounted = true;
-          }
-        }, []);
-
-        return null;
+        let js = String.raw;
+        return (
+          <script
+            nonce={nonce}
+            suppressHydrationWarning
+            dangerouslySetInnerHTML={{
+              __html: js`
+                (() => {
+                  let protocol = location.protocol === "https:" ? "wss:" : "ws:";
+                  let host = location.hostname;
+                  let socketPath = protocol + "//" + host + ":" + ${String(
+                    port
+                  )} + "/socket";
+        
+                  let ws = new WebSocket(socketPath);
+                  ws.onmessage = (message) => {
+                    let event = JSON.parse(message.data);
+                    if (event.type === "LOG") {
+                      console.log(event.message);
+                    }
+                    if (event.type === "RELOAD") {
+                      console.log("💿 Reloading window ...");
+                      window.location.reload();
+                    }
+                  };
+                  ws.onerror = (error) => {
+                    console.log("Remix dev asset server web socket error:");
+                    console.error(error);
+                  };
+                })();
+              `,
+            }}
+          />
+        );
       };
 
 function useComposedRefs<RefValueType = any>(
diff --git a/scripts/release/constants.mjs b/scripts/release/constants.mjs
index b37efcedf..3886eb532 100644
--- a/scripts/release/constants.mjs
+++ b/scripts/release/constants.mjs
@@ -4,15 +4,15 @@ if (!process.env.GITHUB_TOKEN) {
 if (!process.env.GITHUB_REPOSITORY) {
   throw new Error("GITHUB_REPOSITORY is required");
 }
-if (!process.env.GITHUB_REF) {
-  throw new Error("GITHUB_REF is required");
+if (!process.env.VERSION) {
+  throw new Error("VERSION is required");
 }
-if (!process.env.GITHUB_REF.startsWith("refs/tags/")) {
-  throw new Error("GITHUB_REF must be a tag");
+if (!process.env.VERSION.startsWith("refs/tags/")) {
+  throw new Error("VERSION must be a tag, recieved " + process.env.VERSION);
 }
 
 export const [OWNER, REPO] = process.env.GITHUB_REPOSITORY.split("/");
-export const LATEST_RELEASE = process.env.GITHUB_REF.replace("refs/tags/", "");
+export const LATEST_RELEASE = process.env.VERSION.replace("refs/tags/", "");
 export const GITHUB_TOKEN = process.env.GITHUB_TOKEN;
 export const GITHUB_REPOSITORY = process.env.GITHUB_REPOSITORY;
 


Test patch:
diff --git a/packages/remix-react/__tests__/components-test.tsx b/packages/remix-react/__tests__/components-test.tsx
index 53226ed97..c0596735e 100644
--- a/packages/remix-react/__tests__/components-test.tsx
+++ b/packages/remix-react/__tests__/components-test.tsx
@@ -2,7 +2,7 @@ import * as React from "react";
 import { MemoryRouter } from "react-router-dom";
 import { fireEvent, render, act } from "@testing-library/react";
 
-// import type { LiveReload as ActualLiveReload } from "../components";
+import type { LiveReload as ActualLiveReload } from "../components";
 import { Link, NavLink, RemixEntryContext } from "../components";
 
 import "@testing-library/jest-dom/extend-expect";
@@ -15,57 +15,58 @@ import "@testing-library/jest-dom/extend-expect";
 // the browser reloads with the new UI. At the moment we could completely break
 // LiveReload's real features and these tests wouldn't know it.
 
-// describe("<LiveReload />", () => {
-//   const originalNodeEnv = process.env.NODE_ENV;
-//   afterEach(() => {
-//     process.env.NODE_ENV = originalNodeEnv;
-//   });
-
-//   describe("non-development environment", () => {
-//     let LiveReload: typeof ActualLiveReload;
-//     beforeEach(() => {
-//       process.env.NODE_ENV = "not-development";
-//       jest.resetModules();
-//       LiveReload = require("../components").LiveReload;
-//     });
-
-//     it("does nothing if the NODE_ENV is not development", () => {
-//       const { container } = render(<LiveReload />);
-//       expect(container).toBeEmptyDOMElement();
-//     });
-//   });
-
-//   describe("development environment", () => {
-//     let LiveReload: typeof ActualLiveReload;
-//     beforeEach(() => {
-//       process.env.NODE_ENV = "development";
-//       jest.resetModules();
-//       LiveReload = require("../components").LiveReload;
-//     });
-
-//     it("defaults the port to 8002", () => {
-//       const { container } = render(<LiveReload />);
-//       expect(container.querySelector("script")).toHaveTextContent(
-//         /:8002\/socket/
-//       );
-//     });
-
-//     it("can set the port explicitly", () => {
-//       const { container } = render(<LiveReload port={4321} />);
-//       expect(container.querySelector("script")).toHaveTextContent(
-//         /:4321\/socket/
-//       );
-//     });
-
-//     it("determines the right port based on REMIX_DEV_SERVER_WS_PORT env variable", () => {
-//       process.env.REMIX_DEV_SERVER_WS_PORT = "1234";
-//       const { container } = render(<LiveReload />);
-//       expect(container.querySelector("script")).toHaveTextContent(
-//         /:1234\/socket/
-//       );
-//     });
-//   });
-// });
+describe("<LiveReload />", () => {
+  let originalNodeEnv = process.env.NODE_ENV;
+  afterEach(() => {
+    process.env.NODE_ENV = originalNodeEnv;
+  });
+
+  describe("non-development environment", () => {
+    let LiveReload: typeof ActualLiveReload;
+    beforeEach(() => {
+      process.env.NODE_ENV = "not-development";
+      jest.resetModules();
+      LiveReload = require("../components").LiveReload;
+    });
+
+    it("does nothing if the NODE_ENV is not development", () => {
+      let { container } = render(<LiveReload />);
+      expect(container).toBeEmptyDOMElement();
+    });
+  });
+
+  describe("development environment", () => {
+    let oldEnv = process.env;
+    let LiveReload: typeof ActualLiveReload;
+    beforeEach(() => {
+      process.env = { ...oldEnv, NODE_ENV: "development" };
+      jest.resetModules();
+    });
+
+    it("defaults the port to 8002", () => {
+      LiveReload = require("../components").LiveReload;
+      let { container } = render(<LiveReload />);
+      expect(container.querySelector("script")).toHaveTextContent(
+        `8002 + "/socket"`
+      );
+    });
+
+    it("can set the port explicitly", () => {
+      let { container } = render(<LiveReload port={4321} />);
+      expect(container.querySelector("script")).toHaveTextContent(
+        `4321 + "/socket"`
+      );
+    });
+
+    it("determines the right port based on REMIX_DEV_SERVER_WS_PORT env variable", () => {
+      process.env.REMIX_DEV_SERVER_WS_PORT = "1234";
+      let { container } = render(<LiveReload />);
+      expect(container.querySelector("script")).toHaveTextContent(
+        `1234 + "/socket"`
+      );
+    });
+  });
+});
 
 const setIntentEvents = ["focus", "mouseEnter", "touchStart"] as const;
 type PrefetchEventHandlerProps = {