refactor(dev): flatten compiler and use promise-based error handling (#6088)

Author: pcattori

packages/remix-dev/channel.ts

@@ -1,20 +1,28 @@
-export type WriteChannel<T> = {
-  write: (data: T) => void;
-};
-export type ReadChannel<T> = {
-  read: () => Promise<T>;
+type Resolve<V> = (value: V | PromiseLike<V>) => void;
+type Reject = (reason?: any) => void;
+
+type Result<V, E = unknown> = { ok: true; value: V } | { ok: false; error: E };
+
+export type Type<V, E = unknown> = {
+  ok: (value: V) => void;
+  err: (reason?: any) => void;
+  result: Promise<Result<V, E>>;
 };
-export type Channel<T> = WriteChannel<T> & ReadChannel<T>;
 
-export const createChannel = <T>(): Channel<T> => {
-  let promiseResolve: (value: T) => void;
+export const create = <V, E = unknown>(): Type<V> => {
+  let _resolve: Resolve<{ ok: true; value: V }>;
+  let _reject: Reject;
 
-  let promise = new Promise<T>((resolve) => {
-    promiseResolve = resolve;
-  });
+  let promise: Promise<Result<V, E>> = new Promise<Result<V, E>>(
+    (resolve, reject) => {
+      _resolve = resolve;
+      _reject = reject;
+    }
+  ).catch((error) => ({ ok: false, error } as const));
 
   return {
-    write: promiseResolve!,
-    read: () => promise,
+    ok: (value: V) => _resolve!({ ok: true, value } as const),
+    err: _reject!,
+    result: promise,
   };
 };

packages/remix-dev/cli/commands.ts

@@ -168,18 +168,19 @@ export async function build(
   let start = Date.now();
   let config = await readConfig(remixRoot);
   fse.emptyDirSync(config.assetsBuildDirectory);
-  await compiler.build({
-    config,
-    options: {
-      mode,
-      sourcemap,
-      onWarning: warnOnce,
-      onCompileFailure: (failure) => {
-        compiler.logCompileFailure(failure);
-        throw Error();
+  await compiler
+    .build({
+      config,
+      options: {
+        mode,
+        sourcemap,
+        onWarning: warnOnce,
       },
-    },
-  });
+    })
+    .catch((thrown) => {
+      compiler.logThrown(thrown);
+      process.exit(1);
+    });
 
   console.log(`built in ${prettyMs(Date.now() - start)}`);
 }

packages/remix-dev/compiler/assets/index.ts

@@ -1,39 +1,122 @@
-import { createChannel } from "../channel";
-import { type Manifest } from "../manifest";
-import * as AssetsCompiler from "./assets";
+import * as path from "path";
+
 import type { Context } from "./context";
-import * as ServerCompiler from "./server";
+import * as CSS from "./css";
+import * as JS from "./js";
+import * as Server from "./server";
+import * as Channel from "../channel";
+import type { Manifest } from "../manifest";
+import { create as createManifest, write as writeManifest } from "./manifest";
 
 type Compiler = {
-  compile: () => Promise<Manifest | undefined>;
-  dispose: () => void;
+  compile: () => Promise<Manifest>;
+  cancel: () => Promise<void>;
+  dispose: () => Promise<void>;
 };
 
 export let create = async (ctx: Context): Promise<Compiler> => {
+  // channels _should_ be scoped to a build, not a compiler
+  // but esbuild doesn't have an API for passing build-specific arguments for rebuilds
+  // so instead use a mutable reference (`channels`) that is compiler-scoped
+  // and gets reset on each build
   let channels = {
-    manifest: createChannel<Manifest>(),
+    cssBundleHref: undefined as unknown as Channel.Type<string | undefined>,
+    manifest: undefined as unknown as Channel.Type<Manifest>,
+  };
+
+  let subcompiler = {
+    css: await CSS.createCompiler(ctx),
+    js: await JS.createCompiler(ctx, channels),
+    server: await Server.createCompiler(ctx, channels),
   };
 
-  let assets = await AssetsCompiler.create(ctx, channels);
-  let server = await ServerCompiler.create(ctx, channels);
+  let compile = async () => {
+    let hasThrown = false;
+    let cancelAndThrow = async (error: unknown) => {
+      // An earlier error from a failed task has already been thrown; ignore this error.
+      // Safe to cast as `never` here as subsequent errors are only thrown from canceled tasks.
+      if (hasThrown) return undefined as never;
+
+      // resolve channels with error so that downstream tasks don't hang waiting for results from upstream tasks
+      channels.cssBundleHref.err();
+      channels.manifest.err();
+
+      // optimization: cancel tasks
+      subcompiler.css.cancel();
+      subcompiler.js.cancel();
+      subcompiler.server.cancel();
+
+      // Only throw the first error encountered during compilation
+      // otherwise subsequent errors will be unhandled and will crash the compiler.
+      // `try`/`catch` won't handle subsequent errors either, so that isn't a viable alternative.
+      // `Promise.all` _could_ be used, but the resulting promise chaining is complex and hard to follow.
+      hasThrown = true;
+      throw error;
+    };
+
+    // reset channels
+    channels.cssBundleHref = Channel.create();
+    channels.manifest = Channel.create();
+
+    // kickoff compilations in parallel
+    let tasks = {
+      css: subcompiler.css.compile().catch(cancelAndThrow),
+      js: subcompiler.js.compile().catch(cancelAndThrow),
+      server: subcompiler.server.compile().catch(cancelAndThrow),
+    };
+
+    // keep track of manually written artifacts
+    let writes: {
+      cssBundle?: Promise<void>;
+      manifest?: Promise<void>;
+      server?: Promise<void>;
+    } = {};
+
+    // css compilation
+    let css = await tasks.css;
+
+    // css bundle
+    let cssBundleHref =
+      css.bundle &&
+      ctx.config.publicPath +
+        path.relative(
+          ctx.config.assetsBuildDirectory,
+          path.resolve(css.bundle.path)
+        );
+    channels.cssBundleHref.ok(cssBundleHref);
+    if (css.bundle) {
+      writes.cssBundle = CSS.writeBundle(ctx, css.outputFiles);
+    }
+
+    // js compilation (implicitly writes artifacts/js)
+    // TODO: js task should not return metafile, but rather js assets
+    let { metafile, hmr } = await tasks.js;
+
+    // artifacts/manifest
+    let manifest = await createManifest({
+      config: ctx.config,
+      cssBundleHref,
+      metafile,
+      hmr,
+    });
+    channels.manifest.ok(manifest);
+    writes.manifest = writeManifest(ctx.config, manifest);
+
+    // server compilation
+    let serverFiles = await tasks.server;
+    // artifacts/server
+    writes.server = Server.write(ctx.config, serverFiles);
+
+    await Promise.all(Object.values(writes));
+    return manifest;
+  };
   return {
-    compile: async () => {
-      channels.manifest = createChannel();
-      try {
-        let [manifest] = await Promise.all([
-          assets.compile(),
-          server.compile(),
-        ]);
-
-        return manifest;
-      } catch (error: unknown) {
-        ctx.options.onCompileFailure?.(error as Error);
-        return undefined;
-      }
+    compile,
+    cancel: async () => {
+      await Promise.all(Object.values(subcompiler).map((sub) => sub.cancel()));
     },
-    dispose: () => {
-      assets.dispose();
-      server.dispose();
+    dispose: async () => {
+      await Promise.all(Object.values(subcompiler).map((sub) => sub.dispose()));
     },
   };
 };

packages/remix-dev/compiler/compiler.ts

@@ -0,0 +1,60 @@
+import * as path from "path";
+import * as fse from "fs-extra";
+import type * as esbuild from "esbuild";
+import postcss from "postcss";
+import postcssDiscardDuplicates from "postcss-discard-duplicates";
+
+import type { Context } from "../context";
+
+export let write = async (ctx: Context, outputFiles: esbuild.OutputFile[]) => {
+  let cssBundleFile = outputFiles.find((outputFile) =>
+    isBundle(ctx, outputFile, ".css")
+  );
+  if (!cssBundleFile) return;
+
+  let cssBundlePath = cssBundleFile.path;
+
+  let { css, map } = await postcss([
+    // We need to discard duplicate rules since "composes"
+    // in CSS Modules can result in duplicate styles
+    postcssDiscardDuplicates(),
+  ]).process(cssBundleFile.text, {
+    from: cssBundlePath,
+    to: cssBundlePath,
+    map: ctx.options.sourcemap && {
+      prev: outputFiles.find((outputFile) =>
+        isBundle(ctx, outputFile, ".css.map")
+      )?.text,
+      inline: false,
+      annotation: false,
+      sourcesContent: true,
+    },
+  });
+
+  await fse.ensureDir(path.dirname(cssBundlePath));
+
+  await Promise.all([
+    fse.writeFile(cssBundlePath, css),
+    ctx.options.mode !== "production" && map
+      ? fse.writeFile(`${cssBundlePath}.map`, map.toString()) // Write our updated source map rather than esbuild's
+      : null,
+    ...outputFiles
+      .filter((outputFile) => !/\.(css|js|map)$/.test(outputFile.path))
+      .map(async (asset) => {
+        await fse.ensureDir(path.dirname(asset.path));
+        await fse.writeFile(asset.path, asset.contents);
+      }),
+  ]);
+};
+
+export let isBundle = (
+  ctx: Context,
+  outputFile: esbuild.OutputFile,
+  extension: ".css" | ".css.map"
+): boolean => {
+  return (
+    path.dirname(outputFile.path) === ctx.config.assetsBuildDirectory &&
+    path.basename(outputFile.path).startsWith("css-bundle") &&
+    outputFile.path.endsWith(extension)
+  );
+};