refactor(worker): document worker code, improve types (#5034)

This documents a bunch of code related to our node.js worker setup
(which was previously not super well-documented) and also makes a few
improvements to some relevant types.

This was motivated by #5010, which entailed modifying some of this code.
It wasn't entirely apparent how the code in `src/sys/node/worker.ts`,
well, _worked_ and what it was supposed to do. Now we know!

Author: alicewriteswrongs

src/compiler/prerender/prerender-main.ts

@@ -70,8 +70,15 @@ const runPrerender = async (
     let workerCtrl: d.WorkerMainController;
 
     if (config.sys.createWorkerController == null || config.maxConcurrentWorkers < 1) {
+      // we don't have a `maxConcurrentWorkers` setting which makes it
+      // necessary to actually use threaded workers, so we create a
+      // single-threaded worker context here
       workerCtx = createWorkerContext(config.sys);
     } else {
+      // we want to stand up the full threaded worker setup, so we first need
+      // to build a worker controller and then we create an appropriate worker
+      // context for it (this will mean that when methods are called on
+      // `workerCtx` they're dispatched to workers in other threads)
       workerCtrl = config.sys.createWorkerController(config.maxConcurrentWorkers);
       workerCtx = createWorkerMainContext(workerCtrl);
     }

src/compiler/sys/stencil-sys.ts

@@ -622,6 +622,7 @@ export const createSystem = (c?: { logger?: Logger }): CompilerSystem => {
     writeFile,
     writeFileSync,
     generateContentHash,
+    // no threading when we're running in-memory
     createWorkerController: null,
     details: {
       cpuModel: '',

src/compiler/sys/worker/sys-worker.ts

@@ -1,10 +1,19 @@
+import type * as d from '@stencil/core/declarations';
 import { isFunction } from '@utils';
 
-import type { ValidatedConfig } from '../../../declarations';
 import { createWorkerMainContext } from '../../worker/main-thread';
 import { createWorkerContext } from '../../worker/worker-thread';
 
-export const createSysWorker = (config: ValidatedConfig) => {
+/**
+ * Create a worker context given a Stencil config. If
+ * {@link d.Config['maxConcurrentWorkers']} is set to an appropriate value this
+ * will be a worker context that dispatches work to other threads, and if not it
+ * will be a single-threaded worker context.
+ *
+ * @param config the current stencil config
+ * @returns a worker context
+ */
+export const createSysWorker = (config: d.ValidatedConfig): d.CompilerWorkerContext => {
   if (
     isFunction(config.sys.createWorkerController) &&
     config.maxConcurrentWorkers > 0 &&

src/compiler/worker/main-thread.ts

@@ -1,5 +1,14 @@
 import { CompilerWorkerContext, WorkerMainController } from '../../declarations';
 
+/**
+ * Instantiate a worker context which is specific to the 'main thread' and
+ * which dispatches the tasks it receives to a {@link WorkerMainController}
+ * (and, thereby, to workers in other threads).
+ *
+ * @param workerCtrl a worker controller which can handle the methods on the
+ * context by passing them to worker threads
+ * @returns a worker context
+ */
 export const createWorkerMainContext = (workerCtrl: WorkerMainController): CompilerWorkerContext => ({
   optimizeCss: workerCtrl.handler('optimizeCss'),
   prepareModule: workerCtrl.handler('prepareModule'),

src/compiler/worker/worker-thread.ts

@@ -4,22 +4,59 @@ import { prepareModule } from '../optimize/optimize-module';
 import { prerenderWorker } from '../prerender/prerender-worker';
 import { transformCssToEsm } from '../style/css-to-esm';
 
+/**
+ * Instantiate a worker context which synchronously calls the methods that are
+ * defined on it (as opposed to dispatching them to a worker in another thread
+ * via a worker controller). We want to do this in two cases:
+ *
+ * 1. we're in the main thread and not using any workers at all (i.e. the value
+ *    for {@link d.Config['maxConcurrentWorkers']} is set to `1`) or
+ * 2. we're already in a worker thread, so we want to call the method directly
+ *    instead of dispatching.
+ *
+ * @param sys a compiler system appropriate for our current environment
+ * @returns a worker context which directly calls the supported methods
+ */
 export const createWorkerContext = (sys: d.CompilerSystem): d.CompilerWorkerContext => ({
   transformCssToEsm,
   prepareModule,
   optimizeCss,
   prerenderWorker: (prerenderRequest) => prerenderWorker(sys, prerenderRequest),
 });
 
+/**
+ * Create a handler for the IPC messages ({@link d.MsgToWorker}) that a worker
+ * thread receives from the main thread. For each message that we receive we
+ * need to call a specific method on {@link d.CompilerWorkerContext} and then
+ * return the result
+ *
+ * @param sys a compiler system which wil be used to create a worker context
+ * @returns a message handler capable of digesting and executing tasks
+ * described by {@link d.MsgToWorker} object
+ */
 export const createWorkerMessageHandler = (sys: d.CompilerSystem): d.WorkerMsgHandler => {
   const workerCtx = createWorkerContext(sys);
 
-  return (msgToWorker: d.MsgToWorker) => {
-    const fnName: string = msgToWorker.args[0];
-    const fnArgs = msgToWorker.args.slice(1);
-    const fn = (workerCtx as any)[fnName] as Function;
-    if (typeof fn === 'function') {
-      return fn(...fnArgs);
-    }
+  return <T extends d.WorkerContextMethod>(msgToWorker: d.MsgToWorker<T>): ReturnType<d.CompilerWorkerContext[T]> => {
+    const fnName = msgToWorker.method;
+    const fnArgs = msgToWorker.args;
+    const fn = workerCtx[fnName];
+    // This is actually fine! However typescript doesn't agree. Both the
+    // `Parameters` and the `ReturnType` arguments return a union of tuples in
+    // the case where their parameter is generic over some type (e.g. `T` here)
+    // but, annoyingly, TypeScript does not think that you can spread a value
+    // whose type is a union of tuples as a rest param. Even though the type of
+    // `fn` and `fnArgs` should 'match' given the type `T` that they are
+    // generic over, TypeScript does not seem able to realize that and can't
+    // narrow the type of `fn` _or_ `fnArgs` properly, so the type for the
+    // parameters of `fn` _and_ the type of `fnArgs` remains a union of tuples.
+    // Gah!
+    //
+    // See this issue for some context on this:
+    // https://github.com/microsoft/TypeScript/issues/49700
+    //
+    // Unfortunately there's not a great solution here other than:
+    // @ts-ignore
+    return fn(...fnArgs);
   };
 };

src/declarations/stencil-private.ts

@@ -1,4 +1,5 @@
 import { result } from '@utils';
+import type { Serializable as CPSerializable } from 'child_process';
 
 import type { InMemoryFileSystem } from '../compiler/sys/in-memory-fs';
 import type {
@@ -2321,6 +2322,13 @@ export interface VNodeProdData {
   [key: string]: any;
 }
 
+/**
+ * An abstraction to bundle up four methods which _may_ be handled by
+ * dispatching work to workers running in other OS threads or may be called
+ * synchronously. Environment and `CompilerSystem` related setup code will
+ * determine which one, but in either case the call sites for these methods can
+ * dispatch to this shared interface.
+ */
 export interface CompilerWorkerContext {
   optimizeCss(inputOpts: OptimizeCssInput): Promise<OptimizeCssOutput>;
   prepareModule(
@@ -2333,17 +2341,53 @@ export interface CompilerWorkerContext {
   transformCssToEsm(input: TransformCssToEsmInput): Promise<TransformCssToEsmOutput>;
 }
 
-export interface MsgToWorker {
+/**
+ * The methods that are supported on a {@link d.CompilerWorkerContext}
+ */
+export type WorkerContextMethod = keyof CompilerWorkerContext;
+
+/**
+ * A little type guard which will cause a type error if the parameter `T` does
+ * not satisfy {@link CPSerializable} (i.e. if it's not possible to cleanly
+ * serialize it for message passing via an IPC channel).
+ */
+type IPCSerializable<T extends CPSerializable> = T;
+
+/**
+ * A manifest for a job that a worker thread should carry out, as determined by
+ * and dispatched from the main thread. This includes the name of the task to do
+ * and any arguments necessary to carry it out properly.
+ *
+ * This message must satisfy {@link CPSerializable} so it can be sent from the
+ * main thread to a worker thread via an IPC channel
+ */
+export type MsgToWorker<T extends WorkerContextMethod> = IPCSerializable<{
   stencilId: number;
-  args: any[];
-}
+  method: T;
+  args: Parameters<CompilerWorkerContext[T]>;
+}>;
 
-export interface MsgFromWorker {
+/**
+ * A manifest for a job that a worker thread should carry out, as determined by
+ * and dispatched from the main thread. This includes the name of the task to do
+ * and any arguments necessary to carry it out properly.
+ *
+ * This message must satisfy {@link CPSerializable} so it can be sent from the
+ * main thread to a worker thread via an IPC channel
+ */
+export type MsgFromWorker<T extends WorkerContextMethod> = IPCSerializable<{
   stencilId?: number;
-  stencilRtnValue: any;
-  stencilRtnError: string;
-}
+  stencilRtnValue: ReturnType<CompilerWorkerContext[T]>;
+  stencilRtnError: string | null;
+}>;
 
+/**
+ * A description of a task which should be passed to a worker in another
+ * thread. This interface differs from {@link MsgToWorker} in that it doesn't
+ * have to be serializable for transmission through an IPC channel, so we can
+ * hold things like a `resolve` and `reject` callback to use when the task
+ * completes.
+ */
 export interface CompilerWorkerTask {
   stencilId?: number;
   inputArgs?: any[];
@@ -2352,7 +2396,17 @@ export interface CompilerWorkerTask {
   retries?: number;
 }
 
-export type WorkerMsgHandler = (msgToWorker: MsgToWorker) => Promise<any>;
+/**
+ * A handler for IPC messages from the main thread to a worker thread. This
+ * involves dispatching an action specified by a {@link MsgToWorker} object to a
+ * {@link CompilerWorkerContext}.
+ *
+ * @param msgToWorker the message to handle
+ * @returns the return value of the specified function
+ */
+export type WorkerMsgHandler = <T extends WorkerContextMethod>(
+  msgToWorker: MsgToWorker<T>,
+) => ReturnType<CompilerWorkerContext[T]>;
 
 export interface TranspileModuleResults {
   sourceFilePath: string;

src/declarations/stencil-public-compiler.ts

@@ -1027,6 +1027,10 @@ export interface CompilerSystem {
   dynamicImport?(p: string): Promise<any>;
   /**
    * Creates the worker controller for the current system.
+   *
+   * @param maxConcurrentWorkers the max number of concurrent workers to
+   * support
+   * @returns a worker controller appropriate for the current platform (node.js)
    */
   createWorkerController?(maxConcurrentWorkers: number): WorkerMainController;
   encodeToBase64(str: string): string;
@@ -1255,10 +1259,31 @@ export interface ResolveModuleIdResults {
   pkgDirPath: string;
 }
 
+// TODO(STENCIL-1005): improve the typing for this interface
+/**
+ * A controller which provides for communication and coordination between
+ * threaded workers.
+ */
 export interface WorkerMainController {
+  /**
+   * Send a given set of arguments to a worker
+   */
   send(...args: any[]): Promise<any>;
+  /**
+   * Handle a particular method
+   *
+   * @param name of the method to be passed to a worker
+   * @returns a Promise wrapping the results
+   */
   handler(name: string): (...args: any[]) => Promise<any>;
+  /**
+   * Destroy the worker represented by this instance, rejecting all outstanding
+   * tasks and killing the child process.
+   */
   destroy(): void;
+  /**
+   * The current setting for the max number of workers
+   */
   maxWorkers: number;
 }
 

src/sys/node/node-worker-controller.ts

@@ -5,6 +5,10 @@ import { cpus } from 'os';
 import type * as d from '../../declarations';
 import { NodeWorkerMain } from './node-worker-main';
 
+/**
+ * A custom EventEmitter which provides centralizes dispatching and control for
+ * node.js workers ({@link NodeWorkerMain} instances)
+ */
 export class NodeWorkerController extends EventEmitter implements d.WorkerMainController {
   workerIds = 0;
   stencilId = 0;
@@ -15,6 +19,17 @@ export class NodeWorkerController extends EventEmitter implements d.WorkerMainCo
   useForkedWorkers: boolean;
   mainThreadRunner: { [fnName: string]: (...args: any[]) => Promise<any> };
 
+  /**
+   * Create a node.js-specific worker controller, which controls and
+   * coordinates distributing tasks to a series of child processes (tracked by
+   * {@link NodeWorkerMain} instances). These child processes are node
+   * processes executing a special worker script (`src/sys/node/worker.ts`)
+   * which listens for {@link d.MsgToWorker} messages and runs certain tasks in
+   * response.
+   *
+   * @param forkModulePath the path to the module which k
+   * @param maxConcurrentWorkers the max number of worker threads to spin up
+   */
   constructor(
     public forkModulePath: string,
     maxConcurrentWorkers: number,

src/sys/node/node-worker-main.ts

@@ -4,16 +4,31 @@ import { EventEmitter } from 'events';
 
 import type * as d from '../../declarations';
 
+/**
+ * A class that holds a reference to a node worker sub-process within the main
+ * thread so that messages may be passed to it.
+ */
 export class NodeWorkerMain extends EventEmitter {
+  /**
+   * A handle for the OS process that is running our worker code
+   */
   childProcess: cp.ChildProcess;
   tasks = new Map<number, d.CompilerWorkerTask>();
   exitCode: number = null;
   processQueue = true;
-  sendQueue: d.MsgToWorker[] = [];
+  sendQueue: d.MsgToWorker<any>[] = [];
   stopped = false;
   successfulMessage = false;
   totalTasksAssigned = 0;
 
+  /**
+   * Create an object for holding and interacting with a reference to a worker
+   * child-process.
+   *
+   * @param id a unique ID
+   * @param forkModulePath the path to the module which should be run by the
+   * child process
+   */
   constructor(
     public id: number,
     forkModulePath: string,
@@ -60,13 +75,16 @@ export class NodeWorkerMain extends EventEmitter {
     this.totalTasksAssigned++;
     this.tasks.set(task.stencilId, task);
 
+    const [method, ...args] = task.inputArgs;
+
     this.sendToWorker({
       stencilId: task.stencilId,
-      args: task.inputArgs,
+      method,
+      args,
     });
   }
 
-  sendToWorker(msg: d.MsgToWorker) {
+  sendToWorker<T extends d.WorkerContextMethod>(msg: d.MsgToWorker<T>) {
     if (!this.processQueue) {
       this.sendQueue.push(msg);
       return;
@@ -91,7 +109,7 @@ export class NodeWorkerMain extends EventEmitter {
     }
   }
 
-  receiveFromWorker(msgFromWorker: d.MsgFromWorker) {
+  receiveFromWorker<T extends d.WorkerContextMethod>(msgFromWorker: d.MsgFromWorker<T>) {
     this.successfulMessage = true;
 
     if (this.stopped) {