feat(dev): prettier logging for remix build and remix dev (#6596)

remix-run · Jun 14, 2023 · eb06147 · eb06147
1 parent de44305
commit eb06147
Show file tree

Hide file tree

Showing 22 changed files with 362 additions and 303 deletions.
diff --git a/.changeset/orange-beds-explode.md b/.changeset/orange-beds-explode.md
@@ -0,0 +1,5 @@
+---
+"@remix-run/dev": minor
+---
+
+improved logging for `remix build` and `remix dev`
diff --git a/integration/compiler-test.ts b/integration/compiler-test.ts
@@ -428,10 +428,16 @@ test.describe("compiler", () => {
       let importer = path.join("app", "routes", "_index.jsx");
 
       expect(buildOutput).toContain(
-        `The path "some-not-installed-module" is imported in ${importer} but "some-not-installed-module" was not found in your node_modules. Did you forget to install it?`
+        `could not resolve "some-not-installed-module"`
       );
       expect(buildOutput).toContain(
-        `The path "some-not-installed-module/sub" is imported in ${importer} but "some-not-installed-module/sub" was not found in your node_modules. Did you forget to install it?`
+        `You imported "some-not-installed-module" in ${importer},`
+      );
+      expect(buildOutput).toContain(
+        `could not resolve "some-not-installed-module/sub"`
+      );
+      expect(buildOutput).toContain(
+        `You imported "some-not-installed-module/sub" in ${importer},`
       );
     });
   });

diff --git a/integration/esm-only-warning-test.ts b/integration/esm-only-warning-test.ts
@@ -179,25 +179,11 @@ test.beforeAll(async () => {
 });
 
 test("logs warnings for ESM only packages", async () => {
-  expect(buildOutput).toContain(
-    "esm-only-no-exports is possibly an ESM only package"
-  );
-  expect(buildOutput).toContain(
-    "esm-only-exports is possibly an ESM only package"
-  );
-  expect(buildOutput).not.toContain(
-    "esm-only-exports-b is possibly an ESM only package"
-  );
-  expect(buildOutput).not.toContain(
-    "esm-only-exports-c is possibly an ESM only package"
-  );
-  expect(buildOutput).not.toContain(
-    "cjs-dynamic-import is possibly an ESM only package"
-  );
-  expect(buildOutput).toContain(
-    "esm-only-sub-exports is possibly an ESM only package"
-  );
-  expect(buildOutput).not.toContain(
-    "esm-cjs-exports is possibly an ESM only package"
-  );
+  expect(buildOutput).toContain("esm-only package: esm-only-no-exports");
+  expect(buildOutput).toContain("esm-only package: esm-only-exports");
+  expect(buildOutput).not.toContain("esm-only package: esm-only-exports-b");
+  expect(buildOutput).not.toContain("esm-only package: esm-only-exports-c");
+  expect(buildOutput).not.toContain("esm-only package: cjs-dynamic-import");
+  expect(buildOutput).toContain("esm-only package: esm-only-sub-exports");
+  expect(buildOutput).not.toContain("esm-only package: esm-cjs-exports");
 });
diff --git a/integration/flat-routes-test.ts b/integration/flat-routes-test.ts
@@ -5,7 +5,6 @@ import { PlaywrightFixture } from "./helpers/playwright-fixture";
 import type { Fixture, AppFixture } from "./helpers/create-fixture";
 import { createFixtureProject } from "./helpers/create-fixture";
 import { createAppFixture, createFixture, js } from "./helpers/create-fixture";
-import { flatRoutesWarning } from "../packages/remix-dev/config";
 
 let fixture: Fixture;
 let appFixture: AppFixture;
@@ -159,7 +158,7 @@ test.describe("flat routes", () => {
     });
   }
 
-  test("allows ignoredRouteFiles to be configured", async ({ page }) => {
+  test("allows ignoredRouteFiles to be configured", async () => {
     let routeIds = Object.keys(fixture.build.routes);
 
     expect(routeIds).not.toContain(IGNORED_ROUTE);
@@ -210,7 +209,9 @@ test.describe("warns when v1 routesConvention is used", () => {
 
   test("v2_routeConvention is not enabled", () => {
     console.log(buildOutput);
-    expect(buildOutput).toContain(flatRoutesWarning);
+    expect(buildOutput).toContain(
+      "The route file convention is changing in v2"
+    );
   });
 });
 

diff --git a/packages/remix-dev/__tests__/create-test.ts b/packages/remix-dev/__tests__/create-test.ts
@@ -8,14 +8,6 @@ import stripAnsi from "strip-ansi";
 
 import { run } from "../cli/run";
 import { server } from "./msw";
-import {
-  errorBoundaryWarning,
-  flatRoutesWarning,
-  formMethodWarning,
-  headersWarning,
-  metaWarning,
-  serverModuleFormatWarning,
-} from "../config";
 
 beforeAll(() => server.listen({ onUnhandledRequest: "error" }));
 afterAll(() => server.close());
@@ -354,20 +346,8 @@ describe("the create command", () => {
       "--no-install",
       "--no-typescript",
     ]);
-    expect(output.trim()).toBe(
-      errorBoundaryWarning +
-        "\n" +
-        formMethodWarning +
-        "\n" +
-        metaWarning +
-        "\n" +
-        headersWarning +
-        "\n" +
-        serverModuleFormatWarning +
-        "\n" +
-        flatRoutesWarning +
-        "\n\n" +
-        getOptOutOfInstallMessage() +
+    expect(output.trim()).toContain(
+      getOptOutOfInstallMessage() +
         "\n\n" +
         getSuccessMessage(path.join("<TEMP_DIR>", "template-to-js"))
     );

diff --git a/packages/remix-dev/__tests__/readConfig-test.ts b/packages/remix-dev/__tests__/readConfig-test.ts
@@ -1,23 +1,17 @@
 import path from "path";
 
 import type { RemixConfig } from "../config";
-import { serverBuildTargetWarning, readConfig } from "../config";
+import { readConfig } from "../config";
 
 const remixRoot = path.resolve(__dirname, "./fixtures/stack");
 
 describe("readConfig", () => {
   let config: RemixConfig;
-  let warnStub;
   beforeEach(async () => {
-    let consoleWarn = console.warn;
-    warnStub = jest.fn();
-    console.warn = warnStub;
     config = await readConfig(remixRoot);
-    console.warn = consoleWarn;
   });
 
   it("generates a config", async () => {
-    expect(warnStub).toHaveBeenCalledWith(serverBuildTargetWarning);
     expect(config).toMatchInlineSnapshot(
       {
         rootDirectory: expect.any(String),

diff --git a/packages/remix-dev/cli/commands.ts b/packages/remix-dev/cli/commands.ts
@@ -8,6 +8,7 @@ import prettyMs from "pretty-ms";
 import * as esbuild from "esbuild";
 import NPMCliPackageJson from "@npmcli/package-json";
 import { coerce } from "semver";
+import pc from "picocolors";
 
 import * as colors from "../colors";
 import * as compiler from "../compiler";
@@ -23,9 +24,9 @@ import runCodemod from "../codemod";
 import { CodemodError } from "../codemod/utils/error";
 import { TaskError } from "../codemod/utils/task";
 import { transpile as convertFileToJS } from "./useJavascript";
-import { warnOnce } from "../warnOnce";
 import type { Options } from "../compiler/options";
 import { createFileWatchCache } from "../compiler/fileWatchCache";
+import { logger } from "../tux";
 
 export async function create({
   appTemplate,
@@ -152,29 +153,25 @@ export async function build(
 ): Promise<void> {
   let mode = parseMode(modeArg) ?? "production";
 
-  console.log(`Building Remix app in ${mode} mode...`);
+  logger.info(`building...` + pc.gray(` (NODE_ENV=${mode})`));
 
   if (modeArg === "production" && sourcemap) {
-    console.warn(
-      "\n⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️"
-    );
-    console.warn(
-      "You have enabled source maps in production. This will make your " +
-        "server-side code visible to the public and is highly discouraged! If " +
-        "you insist, please ensure you are using environment variables for " +
-        "secrets and not hard-coding them into your source!"
-    );
-    console.warn(
-      "⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️\n"
-    );
+    logger.warn("🚨  source maps enabled in production", {
+      details: [
+        "You are using `--sourcemap` to enable source maps in production,",
+        "making your server-side code publicly visible in the browser.",
+        "This is highly discouraged!",
+        "If you insist, ensure that you are using environment variables for secrets",
+        "and are not hard-coding them in your source.",
+      ],
+    });
   }
 
   let start = Date.now();
   let config = await readConfig(remixRoot);
   let options: Options = {
     mode,
     sourcemap,
-    onWarning: warnOnce,
   };
   if (mode === "development" && config.future.unstable_dev) {
     let origin = await resolveDevOrigin(config);
@@ -184,12 +181,14 @@ export async function build(
   let fileWatchCache = createFileWatchCache();
 
   fse.emptyDirSync(config.assetsBuildDirectory);
-  await compiler.build({ config, options, fileWatchCache }).catch((thrown) => {
-    compiler.logThrown(thrown);
-    process.exit(1);
-  });
+  await compiler
+    .build({ config, options, fileWatchCache, logger })
+    .catch((thrown) => {
+      compiler.logThrown(thrown);
+      process.exit(1);
+    });
 
-  console.log(`Built in ${prettyMs(Date.now() - start)}`);
+  logger.info("built" + pc.gray(` (${prettyMs(Date.now() - start)})`));
 }
 
 // TODO: replace watch in v2
@@ -224,12 +223,12 @@ export async function dev(
     tlsCert?: string;
   } = {}
 ) {
+  // clear screen
+  process.stdout.write("\x1Bc");
+  console.log(`\n 💿  remix dev\n`);
+
   if (process.env.NODE_ENV && process.env.NODE_ENV !== "development") {
-    console.warn(
-      `Forcing NODE_ENV to be 'development'. Was: ${JSON.stringify(
-        process.env.NODE_ENV
-      )}`
-    );
+    logger.warn(`overriding NODE_ENV=${process.env.NODE_ENV} to development`);
   }
   process.env.NODE_ENV = "development";
   if (flags.debug) inspector.open();

diff --git a/packages/remix-dev/compiler/context.ts b/packages/remix-dev/compiler/context.ts
@@ -1,9 +1,11 @@
 import type { RemixConfig } from "../config";
+import type { Logger } from "../tux";
 import type { FileWatchCache } from "./fileWatchCache";
 import type { Options } from "./options";
 
 export type Context = {
   config: RemixConfig;
   options: Options;
   fileWatchCache: FileWatchCache;
+  logger: Logger;
 };
diff --git a/packages/remix-dev/compiler/js/compiler.ts b/packages/remix-dev/compiler/js/compiler.ts
@@ -16,15 +16,10 @@ import { mdxPlugin } from "../plugins/mdx";
 import { externalPlugin } from "../plugins/external";
 import { cssBundlePlugin } from "../plugins/cssBundlePlugin";
 import { cssModulesPlugin } from "../plugins/cssModuleImports";
-import {
-  cssSideEffectImportsPlugin,
-  isCssSideEffectImportPath,
-} from "../plugins/cssSideEffectImports";
+import { cssSideEffectImportsPlugin } from "../plugins/cssSideEffectImports";
 import { vanillaExtractPlugin } from "../plugins/vanillaExtract";
 import invariant from "../../invariant";
 import { hmrPlugin } from "./plugins/hmr";
-import { createMatchPath } from "../utils/tsconfig";
-import { detectPackageManager } from "../../cli/detectPackageManager";
 import type { LazyValue } from "../lazyValue";
 import type { Context } from "../context";
 
@@ -38,21 +33,6 @@ type Compiler = {
   dispose: () => Promise<void>;
 };
 
-function getNpmPackageName(id: string): string {
-  let split = id.split("/");
-  let packageName = split[0];
-  if (packageName.startsWith("@")) packageName += `/${split[1]}`;
-  return packageName;
-}
-
-function isBareModuleId(id: string): boolean {
-  return !id.startsWith("node:") && !id.startsWith(".") && !path.isAbsolute(id);
-}
-
-function isNodeBuiltIn(packageName: string) {
-  return nodeBuiltins.includes(packageName);
-}
-
 const getExternals = (remixConfig: RemixConfig): string[] => {
   // For the browser build, exclude node built-ins that don't have a
   // browser-safe alternative installed in node_modules. Nothing should
@@ -105,18 +85,6 @@ const createEsbuildConfig = (
     );
   }
 
-  let matchPath = ctx.config.tsconfigPath
-    ? createMatchPath(ctx.config.tsconfigPath)
-    : undefined;
-  function resolvePath(id: string) {
-    if (!matchPath) {
-      return id;
-    }
-    return (
-      matchPath(id, undefined, undefined, [".ts", ".tsx", ".js", ".jsx"]) || id
-    );
-  }
-
   let plugins: esbuild.Plugin[] = [
     browserRouteModulesPlugin(ctx, /\?browser$/),
     deprecatedRemixPackagePlugin(ctx),
@@ -135,49 +103,6 @@ const createEsbuildConfig = (
     emptyModulesPlugin(ctx, /\.server(\.[jt]sx?)?$/),
     nodeModulesPolyfillPlugin(),
     externalPlugin(/^node:.*/, { sideEffects: false }),
-    {
-      // TODO: should be removed when error handling for compiler is improved
-      name: "warn-on-unresolved-imports",
-      setup: (build) => {
-        build.onResolve({ filter: /.*/ }, (args) => {
-          if (!isBareModuleId(resolvePath(args.path))) {
-            return undefined;
-          }
-
-          if (args.path === "remix:hmr") {
-            return undefined;
-          }
-
-          let packageName = getNpmPackageName(args.path);
-          let pkgManager = detectPackageManager() ?? "npm";
-          if (
-            ctx.options.onWarning &&
-            !isNodeBuiltIn(packageName) &&
-            !/\bnode_modules\b/.test(args.importer) &&
-            !args.path.endsWith(".css") &&
-            !isCssSideEffectImportPath(args.path) &&
-            // Silence spurious warnings when using Yarn PnP. Yarn PnP doesn’t use
-            // a `node_modules` folder to keep its dependencies, so the above check
-            // will always fail.
-            (pkgManager === "npm" ||
-              (pkgManager === "yarn" && process.versions.pnp == null))
-          ) {
-            try {
-              require.resolve(args.path);
-            } catch (error: unknown) {
-              ctx.options.onWarning(
-                `The path "${args.path}" is imported in ` +
-                  `${path.relative(process.cwd(), args.importer)} but ` +
-                  `"${args.path}" was not found in your node_modules. ` +
-                  `Did you forget to install it?`,
-                args.path
-              );
-            }
-          }
-          return undefined;
-        });
-      },
-    } as esbuild.Plugin,
   ];
 
   if (ctx.options.mode === "development" && ctx.config.future.unstable_dev) {

diff --git a/packages/remix-dev/compiler/options.ts b/packages/remix-dev/compiler/options.ts
@@ -3,7 +3,6 @@ type Mode = "development" | "production" | "test";
 export type Options = {
   mode: Mode;
   sourcemap: boolean;
-  onWarning?: (message: string, key: string) => void;
 
   // TODO: required in v2
   devOrigin?: {

diff --git a/packages/remix-dev/compiler/plugins/deprecatedRemixPackage.ts b/packages/remix-dev/compiler/plugins/deprecatedRemixPackage.ts
@@ -14,13 +14,15 @@ export function deprecatedRemixPackagePlugin(ctx: Context): Plugin {
         // Warn on deprecated imports from the remix package
         if (filePath === "remix") {
           let relativePath = path.relative(process.cwd(), importer);
-          let warningMessage =
-            `WARNING: All \`remix\` exports are considered deprecated as of v1.3.3. ` +
-            `Please change your import in "${relativePath}" to come from the respective ` +
-            `underlying \`@remix-run/*\` package. ` +
-            `Run \`npx @remix-run/dev@latest codemod replace-remix-magic-imports\` ` +
-            `to automatically migrate your code.`;
-          ctx.options.onWarning?.(warningMessage, importer);
+          ctx.logger.warn(`deprecated \`remix\` import in ${relativePath}`, {
+            details: [
+              "Imports from the `remix` package were deprecated in v1.3.3.",
+              "Change your code to import from the appropriate `@remix-run/*` package instead.",
+              "You can run the following codemod to autofix this issue:",
+              "-> `npx @remix-run/dev@latest codemod replace-remix-magic-imports`",
+            ],
+            key: importer,
+          });
         }
         return undefined;
       });