-
Notifications
You must be signed in to change notification settings - Fork 2
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
Make either an array and an object #7
Conversation
Also, @iliaamiri can you see what you think of this? Regardless of the implementation, can you combine some of the go and standard logic into a single location like you did with your utils shit? The core logic should always be the same, just the order of the iterator needs to change |
everythin seems to act the same and the tests still pass, but now inspecting the results with something like console.log doesn't display a bunch of shit about arrays because it's **not** an array anymore
Yes, I"ll look into it |
- clean up console.log and dead comments
@meech-ward |
chore: centralize the createEither utility function
TL;DR The Either object should be simple like this: const either = {
error,
result: undefined,
*[Symbol.iterator]() {
yield error;
yield undefined;
},
} With this simple implementation, the object restructuring works: async function object() {
const { error, result } = await mightFail(Promise.resolve({ message: "success" }))
// error: Error | undefined
// result: {} | undefined
if (error) {
console.error(error)
return
}
// result: {}
console.log(result.message)
} After guarding, the result has the correct type. But with the tuple version async function tuple() {
const [ error, result ] = await mightFail(Promise.resolve({ message: "success" }))
// error: Error | {} | undefined
// result: Error | {} | undefined
if (error) {
console.error(error)
return
}
// result: Error | {} | undefined
console.log(result.message)
} Typescript just can't get the types right unless we make the either object an array. So we had to make export type Either<T> =
| ({
error: Error
result: undefined
} & [Error, undefined])
| ({
result: T
error: undefined
} & [undefined, T]) So now BUT we don't want the either object to show up as an array in the editor or in the logs, so either is still a simple object but proxies all array methods to a proxy function. see It’s proxied so that the object is not an array but just a simple object that appears as a simple object and nothing more Hopefully we can update this when this happens: microsoft/TypeScript#42033 (comment) |
Yeah, hopefully. But I mean, even before we decided to merge arrays and objects together, the "tuple" and "go" versions of the mightFail both were full Arrays anyway. Meaning the user could still access So, I think it's fine to be honest. As long as we don't bring attentions to it, it takes a developer who really digs into everything to recognize it and question it, but I highly doubt it would cause any problems anyway. As long as we keep it simple and don't overstimulate the users with heavy docs, I guess it'll be fine. The current version of our docs are not perfect but they are great enough. |
i can't get this symbol thing working correctly with typescript. it never get's the types perfect, it's always confused about if it's an Error, or T, or undefined. The only way i can get types to be correct is to just use an array.
i can't get this symbol thing working correctly with typescript. it never get's the types perfect, it's always confused about if it's an Error, or T, or undefined. The only way i can get types to be correct is to just use an array.
so that implementation has the object's prototype be the array and the ts type just lies and says it's an array, which means it should always act as an object when using it like an object, but it will act like an array when using it like an array. so
so
for of
would iterate over the array itemsbut
for in
would only iterate over the object items. but you can still call .length and all Array.prototype methods on it and you can index items which i don't love but at least the types work