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

Fix combineWithAllErrors types #483

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

braxtonhall
Copy link

@braxtonhall braxtonhall commented Jun 27, 2023

combineWithAllErrors provides types that are not necessarily correct.

const foo: Result<"a", "b"> = ok("a");
const bar: Result<"c", "d"> = err("d");
Result.combineWithAllErrors([foo, bar]).mapErr((errors) => {
	errors satisfies ["b", "d"]; // should NOT satisfy, but DOES :/
	console.log(errors); // logs ['d'] despite having type ['b', 'd']
	
	const [b] = errors;
	b satisfies "b"; // should NOT satisfy, but DOES :/
	console.log(b); // logs 'd' despite having type "b"
});

The documentation actually correctly matches the implementation, but the type declarations are a little off. In the types, if a literal array is provided, the Results are given to TraverseWithAllErrors which preserves the error order.

@braxtonhall braxtonhall marked this pull request as ready for review June 27, 2023 11:08
@supermacro
Copy link
Owner

#496

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.

None yet

2 participants