feat: add a method for returning an object #10
Conversation
This will allow for handling errors or data using a single reference
|
Thanks for the PR! That being said, I'm not 100% sold on having another function with an ambiguous name, both A questionable alternative I thought about while writing this was having a This would add a couple of ifs inside the function and ship unnecessary code to users that just want one function or the other so I don't really like it. Additionally, users wouldn't be able to choose one or the other by default so they'd be forced to pass the second argument every time if they want the object. Again, don't really like this option... What about having trytm as a wrapper for both and one as a default? This feels better than the other one I wrote above but I'm not 100% sold on this either. In the end, I think it comes down to:
Again, thanks for the PR and I hope we can take this in a direction that makes the lib better :) Any feedback on how to go about this is appreciated |
I totally agree with this, and part of the reason I implemented with this weird-ass name was to spark conversation about the exported methods' names. Most of the time I implemented this exact pattern, I named the function My intuitive response would be call this export const trytm = async <T>(
promise: Promise<T>,
): Promise<TryResult<T>> => {
const getResult = (data: T | null, error: Error | null) => ({
data,
error,
[Symbol.iterator]: function* () {
yield this.data;
yield this.error;
}
})
try {
const data = await promise;
return getResult(data, null);
} catch (throwable) {
if (throwable instanceof Error) {
return getResult(null, throwable);
}
throw throwable;
}
};This would allow for both return types on the same object: const mockPromise = new Promise((resolve) => {
setTimeout(() => {
resolve({ hey: "Bedesqui" });
}, 1000);
})
async function main() {
const [data, error] = await trytm(mockPromise);
const result = await trytm(mockPromise);
}The only bad part about this is that when autocompleting the properties the return object has, it will show:
Which is not pretty... wdyt? |
|
Okay, got a solid implementation of this working class TryResult<T extends any | null> {
private [Symbol.iterator] = function* (): IterableIterator<[T | null, Error | null]> {
yield [this.data, this.error];
}
public data: T | null;
public error: Error | null;
constructor(data: T, error: Error | null) {
this.data = data;
this.error = error;
}
}
export const trytm = async <T>(
promise: Promise<T>,
): Promise<TryResult<T | null>> => {
try {
const data = await promise;
return new TryResult(data, null);
} catch (throwable) {
if (throwable instanceof Error) {
return new TryResult(null, throwable);
}
throw throwable;
}
};
const mockPromise = new Promise<string>((resolve) => {
setTimeout(() => {
resolve('hello');
}, 1000);
})
async function main() {
const [[data, error]] = await trytm(mockPromise);
const result = await trytm(mockPromise);
}This solves |
|
I'm loving the direction this is taking, I'm sold on having both possibilities returned by the same function. I'll try to find a way to not have the |
|
Ha, what a puzzle! The only way I could get it working with both returns with correct return types is with this implementation class TryResult<T extends any | null> {
[Symbol.iterator] = function* () {
yield this.data
yield this.error
return this.error
}
public data: T | null;
public error: Error | null;
constructor(data: T, error: Error | null) {
this.data = data;
this.error = error;
}
}
type TryMixedResult<T> = [T | null, Error | null] & TryResult<T>;
export const trytm = async <T>(
promise: Promise<T>,
): Promise<TryMixedResult<T>> => {
try {
const data = await promise;
return new TryResult(data, null) as any;
} catch (throwable) {
if (throwable instanceof Error) {
return new TryResult(null, throwable) as any;
}
throw throwable;
}
};
const mockPromise = new Promise<string>((resolve) => {
setTimeout(() => {
resolve('hello');
}, 1000);
})
async function main() {
const [data, error] = await trytm(mockPromise);
const result = await trytm(mockPromise);
}But the autocomplete of
|
|
|
|
Here's an alternative approach: type SuccessResult<T, _E> = [T, null] & {
data: T;
error: null
}
type ErrorResult<_T, E> = [null, E] & {
data: null;
error: E
}
type TryResult<T, E> = SuccessResult<T, E> | ErrorResult<T, E>
async function tryTm<T, E = Error>(promise: Promise<T>): Promise<TryResult<T, E>> {
try {
const data = await promise;
const returnable = { data, error: null } as SuccessResult<T, E>;
returnable[0] = data;
returnable[1] = null;
return returnable
} catch (throwable) {
const error = throwable as E;
const returnable = { data: null, error } as ErrorResult<T, E>;
returnable[0] = null;
returnable[1] = error;
return returnable
}
}
const mockPromise = (async () => {
return { greeting: 'Hello' }
})()
async function test() {
const result = await tryTm(mockPromise)
const { data, error } = result;
const [data2, error2] = result;
} |
|
you're a BEAUTIFUL human being |
|
oh, I just tested it; ends up with the same problem I bumped into before :[ |
|
I think at this point we just accept the |
|
@henrycunh Try this type SuccessResult<T, _E> = {
data: T;
error: null;
} & [T, null];
type ErrorResult<_T, E> = {
data: null;
error: E;
} & [null, E];
type TryResult<T, E> = SuccessResult<T, E> | ErrorResult<T, E>;
export async function tryTm<T, E = Error>(
promise: Promise<T>,
): Promise<Omit<TryResult<T, E>, keyof typeof Array.prototype>> {
try {
const data = await promise;
const returnable = [data, null] as SuccessResult<T, E>;
returnable.data = data;
returnable.error = null;
return returnable;
} catch (throwable) {
const error = throwable as E;
const returnable = [null, error] as ErrorResult<T, E>;
returnable.data = null;
returnable.error = error;
return returnable;
}
}
|
|
weird @arn4v just tested here and the array is not showing up, did you paste the right snippet of code? |
|
Omitting keys of Array.prototype also omits Symbol.iterator so TS thinks its no longer destructureable as an Array. (I can still see access by index though result[0]/result[1]) I personally don't care about the autocomplete polution + it makes sense to let it be for technical correctness' sake. Edit: Found a feature request from 7 years ago that was rejected for the same reason (tuples are just arrays, so those methods are supposed to be there): microsoft/TypeScript#6325 (comment). |
|
random question as well, does it matter if you return a tuple or object? why not return an object that satisfies both and the user can just pick depending on how they use it? may also make typing it more straightforward and keep backwards compatibility (not too sure about the practicality of implementing that though) |
In the example I gave with this problem, the return signature of the tuples' items were So, summarizing alternatives:
Those are all the choices, I think |
|
This is not dead! I'm focusing on another project rn but will come back to finish this soon™.
TBH I don't like any of these compromises so I'm leaning toward not implementing this into the lib, and instead adding a function that returns the desired object in the "Hey, this should be copy pasted instead of imported" section of the README. Alternatively, I'm considering dropping the "a single function" part of the lib and exposing this as another import. Maybe something like: import { trytm } from "@bdsqqq/try" // tuple return
import { trytm } from "@bdsqqq/try/tuple" // tuple return
import { trytm } from "@bdsqqq/try/obj" // obj returnThis way we don't have to do any hacky types. Let me know what you think! |
This will allow for handling errors or data using a single reference