arthurfiorette / proposal-safe-assignment-operator

Draft for ECMAScript Error Safe Assignment Operator
https://arthur.run/proposal-safe-assignment-operator/
MIT License
1.38k stars 13 forks source link

Discussion: Preferred operator/keyword for safe assignment #4

Open Not-Jayden opened 2 months ago

Not-Jayden commented 2 months ago

Creating this issue just as a space to collate and continue the discussion/suggestions for the preferred syntax that was initiated on Twitter.

These options were firstly presented on Twitter here:


1. ?= (as !=)

const [error, data] ?= mightFail();
const [error, data] ?= await mightFail();

I generally agreed with this comment on the current proposed syntax:

syntax might be a little too close to the nullish coalescing assignment operator but i love where your head's at; errors as values would be amazing


2. try (as throw)

const [error, data] = try mightFail();
const [error, data] = try await mightFail();

Alternative suggestion for the await case from Twitter here

const [error, data] = await try mightFail();

3. try (as using)

try [error, data] = mightFail();
try [error, data] = await mightFail();

4. ? (as ! in TypeScript)

const [error, data] = mightFail()?;
const [error, data] = await mightFail()?;

👉 Click here to vote 👈


Please feel free to share any other suggestions or considerations :)

vikingair commented 1 week ago

For anyone who might be interested in already "testing" how it feels without waiting for any syntax changes or the proposal to continue, I've created a minimal library with necessary type safety here: https://jsr.io/@backend/safe-assignment

It takes the currently highest voted solution "try (as throw)" and translates from:

const [error, data] = try mightFail();
const [error, data] = try await mightFail();

to

const [error, data] = withErr(mightFail);
const [error, data] = await withErr(mightFail);

I'm already evaluating with it, if it feels better than using the current try-catch, and noted down some caveats.

DScheglov commented 1 week ago

@vikingair I've done the same: https://www.npmjs.com/package/do-try-tuple

By the way, do you consider custom promise classes and different realms (see: isError Proposal)?

vikingair commented 1 week ago

@DScheglov Not yet, as soon as the proposal would be stage 3, and supported by TS, I'd likely migrate from the instanceof check. Thanks for pointing this out.

I also wasn't aware of an existing implementation. The thread here is already quite huge 😬

Looking at your implementation the only differences seem to be:

DScheglov commented 1 week ago

@vikingair

Not yet, as soon as the proposal would be stage 3, and supported by TS, I'd likely migrate from the instanceof check.

The proposal addresses an existing issue: different REALMs (such as iframes and Node.js Virtual Machine modules). Additionally, I encountered a case when jest overrides the global.Error, causing instanceof Error to return a false negative. Yes, it's an edge case, but errors are always on the edge.

I also wasn't aware of an existing implementation. The thread here is already quite huge 😬

There are dozens of similar implementations ) Two most "downloaded":

  • because I wrap anything thrown that isn't an error by an actual error. Hence, I make guarantees on the returned type for errors.

Are you sure that is correct? If someone throws something that isn't an error, do they perhaps expect to catch something that isn't an error (https://github.com/arthurfiorette/proposal-safe-assignment-operator/issues/30#issuecomment-2374912805)? But if you do such transformation, it makes sense to be able detect that and get the original error.

Meaning the following code must work without breaking the code that calls it.

function fn() {
  const [error, result] = try operation();

  if (error) {
    if (error instanceof MyVerySpecificError) return null;
    throw error;
  }

  return 'ok';
}

Now it seems it is better to return the following tuple: [ok: true, error: undefined, result: T] | [ok: false, error: unknown, result: undefined]

Or something like that:

[error: CaughtError, result: undefined] | [error: undegined, result: T] -- See in TS Playground

Because converting an error into something else may result in a different flow splitting behavior than with a standard catch, that can cause some errors will not be handled correctly.

vikingair commented 1 week ago

@DScheglov Thanks for your hints, but so far I created only custom error classes that extend the Error class. Similar to all other native error constructors, e.g. new TypeError() instanceof Error === true.

Wrapping the thrown thing into an error is mainly done to push best practices of avoiding to throw anything that isn't an error. I know it can cause issues with a lot of existing code.

My implementation is just "one" implementation. Very likely not the best or most appropriate. Shouldn't be taken as a guideline implementation for this proposal but rather to get a feeling of how it is to write JS following that syntax. Getting a feeling of it feels weird or cumbersome to write JS error handling like that.

DScheglov commented 1 week ago

@vikingair

Wrapping the thrown thing into an error is mainly done to push best practices of avoiding to throw anything that isn't an error. I know it can cause issues with a lot of existing code.

If something can cause the issues with existing code how it could be a best practice? :) Well, to throw only instances of Error classes -- is a best practice. Wrapping caught non-error value is a different thing, especially considering that also an error could be falsy treated as non-error.

In any case, if you wrap something, please ensure that someone can unwrap this something.

bparks commented 11 hours ago

I'm glad to see a lot of momentum around the "try as throw" syntax proposal (... = try doSomething()).

However, I think it's problematic to "return" a tuple in an attempt to mirror Go's syntax. Any functional language (or Rust) would be a better example, IMO feeling far more idiomatic.

Something like:

let processedResult;
const maybeResult = try doSomething();
if (maybeResult.ok) {
  processedResult = doSomethingWithResult(maybeResult.result);
} else {
  processedResult = doSomethingElseInstead(maybeResult.error);
}

I don't feel like this, by itself, has much of an advantage over a traditional try...catch, especially if/when a finally is involved, but it does open up additional possibilities that a tuple just wouldn't, like:

const processedResult = (try doSomething())
  .unwrapOrElse(doSomethingElseInstead)
  .then(doSomethingWithResult)
  .finally(...)

(Note: I used unwrapOrElse to mirror a similarly-named Rust function, but not to indicate a strong opinion on what the name should be)

To be fair, I'm not sold on introducing another meaning/purpose for then (but I can't think of a better word right now) or the encouraged return to method-chaining that async/await alleviated for Promises, but doing something like the following also feels weird (two try keywords):

try {
  const result = (try doSomething())
    .unwrapOrElse(doSomethingElseInstead);
  const processedResult = doSomethingWithResult(result);
} finally {
  //...
}
DScheglov commented 2 hours ago

@bparks

It's a great idea.

However, this means we also need to introduce the Result<T, E> type with a rich API—far richer than what promises offer. Alternatively, we need a way to specify a concrete type to be returned from a try-expression, allowing developers to use their own implementations.

Additionally to Result<T, E>, imho, it makes sense to introduce the Symbol.result to allow implementaion of

interface Resultable<T, E> {
  [Symbol.result](): { value: T, ok: true } | { error: E; ok: false }
}

And two operators: unwrap and unwrap! (not final naming as well) to get able to write code like this:

declare async function createUser(userData: UserData): Promise<Result<User, CreateUserError>>;

async function signUp(userData: UserData) {
  // --- snip ---
  const user = unwrap await createUser(userData);
  // assigns user with value of type User OR
  // returns Result<never, CreateUserError> (wrapped to Promise.resolve)
}

The code bases that:

class Result<T,E> implements Resultable<T, Result<never, E>> {
 // -- snip --
}

The unwrap! operators just throw the error, recieved from [Symbol.result]() call instead of the returning it

@ljharb , @arthurfiorette what do you think about such approach?

ljharb commented 9 minutes ago

I’m not clear on why a symbol would be needed?