Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Result.all accepting spread parameters encourages situations for stack overflows #85

Open
ConnorSinnott opened this issue Mar 27, 2024 · 1 comment

Comments

@ConnorSinnott
Copy link

ConnorSinnott commented Mar 27, 2024

Hello! I'm curious about the backstory behind why Result.all takes the pattern of

export function all<T extends Result<any, any>[]>(
    ...results: T
): Result<ResultOkTypes<T>, ResultErrTypes<T>[number]> {

rather than

export function all<T extends Result<any, any>[]>(
    results: T
): Result<ResultOkTypes<T>, ResultErrTypes<T>[number]> {

I understand that for situations where you have one or two results assigned to variables, its easier to write Result.all(res1, res2) than Result.all([res1, res2]) but having used this library in an enterprise environment, its far far more common that we are processing an array of results, leaving the code littered with Result.all(...resultArray).

For a while, this issue was mitigated since we can just spread the array in, as I'm sure was part of the consideration when choosing this function signature. However, if the array of results grows large enough, spreading the array into the parameters could lead to a stack overflow. This had us implement our own version of Result.all which accepts an array, but otherwise is virtually identical.

I think it's a bit of a code smell to re-write tools that come out of the box. Would it be possible to have another variant of Result.all added to the library which accepts an array rather than a spread parameter.

@ConnorSinnott ConnorSinnott changed the title Result.all accepting spread parameters leads to situations for stack overflows Result.all accepting spread parameters encourages situations for stack overflows Mar 27, 2024
@ConnorSinnott
Copy link
Author

ConnorSinnott commented Aug 26, 2024

This repo appears dead, so this suggestion has been implemented in lune-climate#125

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant