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

Correct signature for Result.partition #172

Merged

Conversation

ConnorSinnott
Copy link

@ConnorSinnott ConnorSinnott commented Sep 7, 2024

Hello @jstasiak! I must confess, I hadn't been able to update to the latest version of ts-results-es until recently. I had a shower thought last night that the signature for Result.partition was actually incorrect.

const all1 = Result.partition([ok0, ok1, err0, err1]);
expect(all1).toEqual([
    [ok0.value, ok1.value],
    [err0.error, err1.error],
]);
eq<typeof all1, [[number, boolean, never, never], [never, never, symbol, Error]]>(true);

The signature above implies that all is equal to [[number, boolean, undefined, undefined], [undefined, undefined, symbol, Error]] but this is not how that function works. Successes and errors will be pushed onto the partition array as they come, and we can't guarantee what index the successes or errors will be at. The real result from that function would be [[number, boolean], [symbol, Error]]

I've updated the signature and tests to reflect this. Could we patch this in? Very sorry for the inconvenience.

@ConnorSinnott ConnorSinnott force-pushed the correct-partition-signature branch 2 times, most recently from 2842549 to a048567 Compare September 7, 2024 18:26
Copy link
Collaborator

@jstasiak jstasiak left a comment

Choose a reason for hiding this comment

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

Good stuff, thank you.

Nothing to be sorry about!

@jstasiak jstasiak merged commit 6fd7d43 into lune-climate:master Sep 11, 2024
2 checks passed
@ConnorSinnott ConnorSinnott deleted the correct-partition-signature branch September 11, 2024 18:55
@ConnorSinnott
Copy link
Author

Thank you @jstasiak! When can we expect this to be published? The 4.2.0 version has a bugged signature so to limit impact I think we'd want to patch this at your earliest convenience.

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.

2 participants