From 4538cbfe28fdb66b6c4a8ff92de6d1f307812b6f Mon Sep 17 00:00:00 2001 From: NKDuy Date: Fri, 1 Dec 2023 18:47:07 +0700 Subject: [PATCH] refactor(compiler): add a Result union type for polymorphic returns --- scripts/build.ts | 2 + scripts/bundles/internal.ts | 6 +- scripts/bundles/utils.ts | 44 ++++++++ src/cli/find-config.ts | 13 ++- src/cli/run.ts | 9 +- src/compiler/build/build-ctx.ts | 4 +- src/compiler/build/build-stats.ts | 39 ++++---- src/declarations/rindo-private.ts | 4 +- src/utils/index.ts | 1 + src/utils/result.ts | 160 ++++++++++++++++++++++++++++++ 10 files changed, 246 insertions(+), 36 deletions(-) create mode 100644 scripts/bundles/utils.ts create mode 100644 src/utils/result.ts diff --git a/scripts/build.ts b/scripts/build.ts index b485f985..2118f626 100644 --- a/scripts/build.ts +++ b/scripts/build.ts @@ -10,6 +10,7 @@ import { mockDoc } from './bundles/mock-doc'; import { screenshot } from './bundles/screenshot'; import { sysNode, sysNodeExternalBundles } from './bundles/sys-node'; import { testing } from './bundles/testing'; +import { utils } from './bundles/utils'; import { createLicense } from './license'; import { release } from './release'; import { validateBuild } from './test/validate-build'; @@ -66,6 +67,7 @@ export async function createBuild(opts: BuildOptions): Promise { + const outputDirPath = join(opts.output.internalDir, 'utils'); + await fs.ensureDir(outputDirPath); + + // copy the `.d.ts` file corresponding to `src/utils/result.ts` + const resultDtsFilePath = join(opts.buildDir, 'utils', 'result.d.ts'); + const resultDtsOutputFilePath = join(opts.output.internalDir, 'utils', 'result.d.ts'); + await fs.copyFile(resultDtsFilePath, resultDtsOutputFilePath); + + const utilsIndexDtsPath = join(opts.output.internalDir, 'utils', 'index.d.ts'); + // here we write a simple module that re-exports `./result` so that imports + // elsewhere like `import { result } from '@utils'` will resolve correctly + await fs.writeFile(utilsIndexDtsPath, `export * as result from "./result"`); +}; diff --git a/src/cli/find-config.ts b/src/cli/find-config.ts index 1b7b61c0..df8b1414 100644 --- a/src/cli/find-config.ts +++ b/src/cli/find-config.ts @@ -1,4 +1,4 @@ -import { buildError, isString, normalizePath } from '@utils'; +import { buildError, isString, normalizePath, result } from '@utils'; import type { CompilerSystem, Diagnostic } from '../declarations'; @@ -17,7 +17,6 @@ export type FindConfigOptions = { export type FindConfigResults = { configPath: string; rootDir: string; - diagnostics: Diagnostic[]; }; /** @@ -25,7 +24,7 @@ export type FindConfigResults = { * @param opts the options needed to find the configuration file * @returns the results of attempting to find a configuration file on disk */ -export const findConfig = async (opts: FindConfigOptions): Promise => { +export const findConfig = async (opts: FindConfigOptions): Promise> => { const sys = opts.sys; const cwd = sys.getCurrentDirectory(); const rootDir = normalizePath(cwd); @@ -49,16 +48,16 @@ export const findConfig = async (opts: FindConfigOptions): Promise { startupLog(logger, task); const findConfigResults = await findConfig({ sys, configPath: flags.config }); - if (hasError(findConfigResults.diagnostics)) { - logger.printDiagnostics(findConfigResults.diagnostics); + if (findConfigResults.isErr) { + logger.printDiagnostics(findConfigResults.value); return sys.exit(1); } @@ -67,11 +67,12 @@ export const run = async (init: d.CliInitOptions) => { return; } + const foundConfig = result.unwrap(findConfigResults); const validated = await coreCompiler.loadConfig({ config: { flags, }, - configPath: findConfigResults.configPath, + configPath: foundConfig.configPath, logger, sys, }); diff --git a/src/compiler/build/build-ctx.ts b/src/compiler/build/build-ctx.ts index 9da4d95b..1ce95763 100644 --- a/src/compiler/build/build-ctx.ts +++ b/src/compiler/build/build-ctx.ts @@ -1,4 +1,4 @@ -import { hasError, hasWarning } from '@utils'; +import { hasError, hasWarning, result } from '@utils'; import type * as d from '../../declarations'; @@ -18,7 +18,7 @@ export class BuildContext implements d.BuildCtx { componentGraph = new Map(); config: d.Config; data: any = {}; - buildStats?: d.CompilerBuildStats = undefined; + buildStats?: result.Result = undefined; esmBrowserComponentBundle: d.BundleModule[]; esmComponentBundle: d.BundleModule[]; es5ComponentBundle: d.BundleModule[]; diff --git a/src/compiler/build/build-stats.ts b/src/compiler/build/build-stats.ts index 876cdca4..eba85728 100644 --- a/src/compiler/build/build-stats.ts +++ b/src/compiler/build/build-stats.ts @@ -1,4 +1,4 @@ -import { byteSize, isOutputTargetStats, sortBy } from '@utils'; +import { byteSize, isOutputTargetStats, result, sortBy } from '@utils'; import type * as d from '../../declarations'; @@ -11,17 +11,15 @@ import type * as d from '../../declarations'; export function generateBuildStats( config: d.Config, buildCtx: d.BuildCtx -): d.CompilerBuildStats | { diagnostics: d.Diagnostic[] } { +): result.Result { // TODO(RINDO-461): Investigate making this return only a single type const buildResults = buildCtx.buildResults; - let jsonData: d.CompilerBuildStats | { diagnostics: d.Diagnostic[] }; - try { if (buildResults.hasError) { - jsonData = { + return result.err({ diagnostics: buildResults.diagnostics, - }; + }); } else { const stats: d.CompilerBuildStats = { timestamp: buildResults.timestamp, @@ -58,8 +56,7 @@ export function generateBuildStats( rollupResults: buildCtx.rollupResults, collections: getCollections(config, buildCtx), }; - - jsonData = stats; + return result.ok(stats); } } catch (e: unknown) { const diagnostic: d.Diagnostic = { @@ -68,12 +65,10 @@ export function generateBuildStats( messageText: `Generate Build Stats Error: ` + e, type: `build`, }; - jsonData = { + return result.err({ diagnostics: [diagnostic], - }; + }); } - - return jsonData; } /** @@ -84,19 +79,21 @@ export function generateBuildStats( */ export async function writeBuildStats( config: d.Config, - data: d.CompilerBuildStats | { diagnostics: d.Diagnostic[] } + data: result.Result ): Promise { const statsTargets = config.outputTargets.filter(isOutputTargetStats); - await Promise.all( - statsTargets.map(async (outputTarget) => { - const result = await config.sys.writeFile(outputTarget.file, JSON.stringify(data, null, 2)); + await result.map(data, async (compilerBuildStats) => { + await Promise.all( + statsTargets.map(async (outputTarget) => { + const result = await config.sys.writeFile(outputTarget.file, JSON.stringify(compilerBuildStats, null, 2)); - if (result.error) { - config.logger.warn([`Stats failed to write file to ${outputTarget.file}`]); - } - }) - ); + if (result.error) { + config.logger.warn([`Stats failed to write file to ${outputTarget.file}`]); + } + }) + ); + }); } function sanitizeBundlesForStats(bundleArray: ReadonlyArray): ReadonlyArray { diff --git a/src/declarations/rindo-private.ts b/src/declarations/rindo-private.ts index 78c481b6..8d1d015a 100644 --- a/src/declarations/rindo-private.ts +++ b/src/declarations/rindo-private.ts @@ -1,3 +1,5 @@ +import { result } from '@utils'; + import type { InMemoryFileSystem } from '../compiler/sys/in-memory-fs'; import type { BuildEvents, @@ -207,7 +209,7 @@ export interface UpdatedLazyBuildCtx { export interface BuildCtx { buildId: number; buildResults: CompilerBuildResults; - buildStats?: CompilerBuildStats | { diagnostics: Diagnostic[] }; + buildStats?: result.Result; buildMessages: string[]; bundleBuildCount: number; collections: Collection[]; diff --git a/src/utils/index.ts b/src/utils/index.ts index e7c2ace7..615f9985 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -11,6 +11,7 @@ export * from './message-utils'; export * from './output-target'; export * from './path'; export * from './query-nonce-meta-tag-content'; +export * as result from './result'; export * from './sourcemaps'; export * from './url-paths'; export * from './util'; diff --git a/src/utils/result.ts b/src/utils/result.ts new file mode 100644 index 00000000..5b709a4a --- /dev/null +++ b/src/utils/result.ts @@ -0,0 +1,160 @@ +/** + * A Result wraps up a success state and a failure state, allowing you to + * return a single type from a function and discriminate between the two + * possible states in a principled way. + * + * Using it could look something like this: + * + * ```ts + * import { result } from '@utils'; + * + * const mightFail = (input: number): Result => { + * try { + * let value: number = calculateSomethingWithInput(input); + * return result.ok(value); + * } catch (e) { + * return result.err(e.message); + * } + * } + * + * const sumResult = mightFail(2); + * + * const msg = result.map(sumResult, (sum: number) => `the sum was: ${sum}`); + * ``` + * + * A few utility methods are defined in this module, like `map` and `unwrap`, + * which are (probably obviously) inspired by the correspond methods on + * `std::result::Result` in Rust. + */ +export type Result = Ok | Err; + +/** + * Type for the Ok state of a Result + */ +type Ok = { + isOk: true; + isErr: false; + value: T; +}; + +/** + * Type for the Err state of a Result + */ +type Err = { + isOk: false; + isErr: true; + value: T; +}; + +/** + * Create an `Ok` given a value. This doesn't do any checking that the value is + * 'ok-ish' since doing so would make an undue assumption about what is 'ok'. + * Instead, this trusts the user to determine, at the call site, whether + * something is `ok()` or `err()`. + * + * @param value the value to wrap up in an `Ok` + * @returns an Ok wrapping the value + */ +export const ok = (value: T): Ok => ({ + isOk: true, + isErr: false, + value, +}); + +/** + * Create an `Err` given a value. + * + * @param value the value to wrap up in an `Err` + * @returns an Ok wrapping the value + */ +export const err = (value: T): Err => ({ + isOk: false, + isErr: true, + value, +}); + +/** + * Map across a `Result`. + * + * If it's `Ok`, unwraps the value, applies the supplied function, and returns + * the result, wrapped in `Ok` again. This could involve changing the type of + * the wrapped value, for instance: + * + * ```ts + * import { result } from "@utils"; + * + * const myResult: Result = result.ok("monads???"); + * const updatedResult = result.map(myResult, wrappedString => ( + * wrappedString.split("").length + * )); + * ``` + * + * after the `result.map` call the type of `updatedResult` will now be + * `Result`. + * + * If it's `Err`, just return the same value. + * + * This lets the programmer trigger an action, or transform a value, only if an + * earlier operation succeeded, short-circuiting instead if an error occurred. + * + * @param result a `Result` value which we want to map across + * @param fn a function for handling the `Ok` case for the `Result` + * @returns a new `Result`, with the a new wrapped value (if `Ok`) or the + * same (if `Err) + */ +export function map(result: Result, fn: (t: T1) => Promise): Promise>; +export function map(result: Result, fn: (t: T1) => T2): Result; +export function map( + result: Result, + fn: ((t: T1) => T2) | ((t: T1) => Promise) +): Promise> | Result { + if (result.isOk) { + const val = fn(result.value); + if (val instanceof Promise) { + return val.then((newVal) => ok(newVal)); + } else { + return ok(val); + } + } + + if (result.isErr) { + // unwrapping the error is necessary here for typechecking + // but you and I both know its type hasn't changed a bit! + const value = result.value; + return err(value); + } + + throw 'should never get here'; +} + +/** + * Unwrap a {@link Result}, return the value inside if it is an `Ok` and + * throw with the wrapped value if it is an `Err`. + * + * @throws with the wrapped value if it is an `Err`. + * @param result a result to peer inside of + * @returns the wrapped value, if `Ok` + */ +export const unwrap = (result: Result): T => { + if (result.isOk) { + return result.value; + } else { + throw result.value; + } +}; + +/** + * Unwrap a {@link Result}, return the value inside if it is an `Err` and + * throw with the wrapped value if it is an `Ok`. + * + * @throws with the wrapped value if it is an `Ok`. + * @param result a result to peer inside of + * @returns the wrapped value, if `Err` + */ +export const unwrapErr = (result: Result): E => { + if (result.isErr) { + return result.value; + } else { + throw result.value; + } +};