feat: replacing "tuple" implementation of Result with monomorphic object

[jake-danton]

Summary

The Result type is now a monomorphic object rather than a "tuple". This has performance, functionality, and ergonomic improvements.

Reasons:

• Better performance: Monomorphic objects are more performant than tuples or polymorphic objects in most JavaScript engines (they optimize around functions having the same return shape, etc). Also the overhead of calling isResultOk or isResultErr when you already knew it was a Result was very high and to be avoided.
• More flexibility: This implementation has no restrictions on the types for TOk or TErr. Even Result<undefined, undefined> is now valid.
• Improved developer experience: Self-documenting properties (result.ok, result.value, result.error) are more readable than array indices
• Type safety: Eliminates confusion about tuple ordering and reduces potential for accessing wrong array positions or accidentally treating an array as a result and vice versa.
• Consistency: Aligns with Result patterns used in other languages like Rust

Syntax Comparison:

// using current tuple implementation
function doThing(): Result<number, Error> {
    if (Math.random() > 0.5) {
        return [undefined, 42]; // these being flipped from the type argument order is confusing!
    } else {
        return [new Error("Something went wrong"), undefined];
    }
}

const result = doThing();
// Using `isResultOk(result)` works here, but is much slower
if (result[0] === undefined) {
    console.log("Success:", result[1]);
} else {
    console.error("Error:", result[0]);
}

// Using new monomorphic object implementation
function doThing(): _.Result<number, string> {
    if (Math.random() > 0.5) {
        return _.ok(42);
    } else {
        return _.err("Something went wrong");
    }
}

const result = doThing();
if (result.ok) {
    console.log("Success:", result.value);
} else {
    console.error("Error:", result.error);
}

And now, if you want, you can drop the heavier Error object (which can make it 1000x slower than using number or string or other) and even used undefined for your TErr:

function parse<T>(input: string): _.Result<T, undefined> {
  //...
}

const result = parse<number |  undefined>('42')
const value = result.ok ? result.value : 0 // if *why* it fails never matters and `undefined` is a valid response.

Benchmarks:
I created a comparison benchmark test you can copy-paste into this repo to explore the results since benchmarks are finicky and easy to get wrong.

The results are better in every case for monomorphic objects over tuples. The improvement to create them is negligible (~5%) but the difference in checking success and getting values is ~2x faster than indexing into the tuple (and >300x faster than using isResultOk/Err!), and ~4x faster to run isResult.

Related issue, if any:

For any code change,

• Related documentation has been updated, if needed
• Related tests have been added or updated, if needed
• Related benchmarks have been added or updated, if needed
• Release notes in next-minor.md or next-major.md have been added, if needed

Does this PR introduce a breaking change?

Yes

All uses of the _.Result will have to be updated to use the object definition instead of the tuple one.

Bundle impact

Status	File	Size 1	Difference
M	src/async/defer.ts	518	+103 (+25%)
M	src/async/parallel.ts	1800	+85 (+5%)
M	src/async/retry.ts	634	+99 (+19%)
M	src/async/toResult.ts	253	+36 (+17%)
M	src/async/tryit.ts	316	+76 (+32%)
A	src/typed/err.ts	67	+67
M	src/typed/isResult.ts	169	-48 (-22%)
A	src/typed/ok.ts	66	+66

Footnotes

1. Function size includes the import dependencies of the function. ↩

[radashi-bot]

Benchmark Results

Name	Current	Baseline	Change
isResult ▶︎ with valid Ok value	4,681,384.9 ops/sec ±0.1%	4,816,096.71 ops/sec ±0.14% 🔗	🐢 -2.8%
isResult ▶︎ with valid Err value	4,666,908.53 ops/sec ±0.1%	4,767,093 ops/sec ±0.09% 🔗	🐢 -2.1%
isResult ▶︎ with invalid value	4,610,339.85 ops/sec ±1.68%	4,776,820.39 ops/sec ±0.09% 🔗	🐢 -3.49%

Performance regressions of 30% or more should be investigated, unless they were anticipated. Smaller regressions may be due to normal variability, as we don't use dedicated CI infrastructure.

[aleclarson]

Hi Jake, thanks for the PR!

I'm leaning toward rejecting this change. Let me start with my biggest subjective objection: I find the array destructuring more ergonomic; and you get used to the error being the first element.

I also think the performance boost of the object approach is over-emphasized. You're not using isResult or its Ok/Err variants all that often, in practice, which I assume is slower than your implementation because of the isError() check. Instead, you typically do if (error), which is blazingly fast.

While object properties are easier to read than array indexed access, the idiomatic pattern is immediate array destructuring, which is plenty readable:

const [error, value] = myResultFunc()
if (error) {
  return doSomethingWithError(error)
}
doSomethingWithValue(value)

As far as Err results with a non-Error type, I'd call that “bad practice” and something we shouldn't encourage. If you need the performance boost of avoiding new Error allocation, you're perfectly free to use a custom tuple type like [error: string, result: null] | [error: null, result: T], but I wonder if that's really where your bottleneck resides. Most of the time, your program should not be producing errors anyway.

Curious to hear your thoughts. Thanks again for proposing this change. To be clear, I'm absolutely open to the idea, but I have my reservations. I'm also curious to hear what others have to say on this, if anyone else wants to chime in. ✌️

[jake-danton]

No worries! I understand if your vision of Result and mine differ too much to merge it. I just thought I'd make the PR as I think the monomorphic version is better in every way (sorry if that reads like I am trying to diss the tuple version as I am not; I just have a strong opinion on type-safe error handling! 😁). I was utilizing Radashi in a project for the first time and realized I couldn't use the _.Result over my version as it is not type-safe or performant enough, so thought I would see if people were on board with the version I have in my personal utility library for the reasons I laid out.

The tuple version with destructuring is more like Go than Rust and a lot of people (probably most) like that style more. I don't, but I also like Rust and have super strict TS linting turned on to be aggressively opinionated at compile time, so I already know my opinion on these things does not reflect the popular perspective.

Otherwise, Radashi has been great to work with so far! 🫜

[jake-danton]

And on the note of requiring Error, I don't see clear value add and it forces overhead in code and worse runtime performance. Strongly-typed errors are important but Error doesn't even enforce that as you can just have new Error() and it takes it just fine. Whereas for example:

// This non-Error type has all the real error info
type ValidationError =
  | {
    type: "tooLow";
    minCutoff: number;
  }
  | {
    type: "tooHigh";
    maxCutoff: number;
  };
  
  // This wrapper Error adds nothing
  class ValidationErrorClass extends Error {
  constructor(public validationError: ValidationError) {
    super(`Validation error: ${JSON.stringify(validationError)}`);
  }
}

And you can still enforce not having a TErr that is undefined by doing something like:

type Result<TOk, TErr> = TErr extends undefined
  ? [never, TOk]
  : [TErr, undefined] | [undefined, TOk];
  
 type DumbResult = Result<number, undefined>; // type is `[never, number]`