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

Add non-spread overload for Result.all #86

Closed
wants to merge 1 commit into from
Closed

Add non-spread overload for Result.all #86

wants to merge 1 commit into from

Conversation

ConnorSinnott
Copy link

This PR adds non-spread variants of Result.all and Result.any which should be preferred over the parameter spread variants. For instances where Result.all or Result.any are invoked with very large arrays, the spread operation could lead to stack overflows. Additionally, though heavy usage of Result.all in an enterprise enviornment, usage of Result.all with an array (Result.all(...myResultArray)) grossly exceeds the times where Result.all is used with multiple named results passed as individual arguments.

To accurately support the inferred types from Result.all and Result.any, const type parameters were used which requires bumping up to at least Typescript 5.0.

This PR solves #85

'^.*\.ts$': ['ts-jest', {
tsconfig: 'test/tsconfig.json'
}]
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This silences the warnings that global ts-jest configuration is deprecated

"typescript": "^4.2"
"ts-jest": "^29.1.2",
"tslib": "^2.6.2",
"typescript": "^5.4"
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bumping to typescript 5.X allows the use of const type params.

// Utility types
type Head<T extends any[]> = T extends [any, ...infer R] ?
T extends [...infer F, ...R] ? F : never : never
type Tail<T extends any[]> = T extends [any, ...infer R] ? R : never
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These could likely be removed since the arrays I'm parsing above are typed as any[]. I could just as easily type the arguments as arg0: T[0] | T, argN: ...T but using these types are technically truer to what's going on.

expect(all1Array).toMatchResult(Ok([3, true]));
eq<typeof all1Array, Result<[number, boolean], never>>(true);

const all2 = Result.all(err0, err1);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected the naming indexes being off for all2 and onward

@ConnorSinnott
Copy link
Author

Hey @kevinsimper! Looks like you're somewhat active on this repo. Mind taking a peek at this?

@ConnorSinnott
Copy link
Author

Or @vultix are you still around?

@ConnorSinnott
Copy link
Author

I'd like to have #81 merged in as well since I'm fighting errors with expectErr

@ConnorSinnott ConnorSinnott closed this by deleting the head repository Apr 16, 2024
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

Successfully merging this pull request may close these issues.

1 participant