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

re: Why Not data First? #13

Open
smeijer opened this issue Aug 16, 2024 · 11 comments
Open

re: Why Not data First? #13

smeijer opened this issue Aug 16, 2024 · 11 comments
Labels
documentation Improvements or additions to documentation

Comments

@smeijer
Copy link

smeijer commented Aug 16, 2024

Why Not data First?
In Go, the convention is to place the data variable first, and you might wonder why we don't follow the same approach in JavaScript. In Go, this is the standard way to call a function. However, in JavaScript, we already have the option to use const data = fn() and choose to ignore the error, which is precisely the issue we are trying to address.

I believe there's another reason to stick to error first in js, and that's for language consistency in Nodejs, the most used JS server runtime. When using callback functions, the convention is to use an error first pattern. Like node's new glob util:

import { glob } from 'node:fs';

glob('**/*.js', (err, matches) => {
  if (err) throw err;
  console.log(matches);
});

Mapping that to the following feels almost natural.

const [err, matches] ?= glob('**/.js');
if (err) throw err;
console.log(matches);

I think the error-first callback pattern is popular enough, even in libraries, to support this case of error first, and it might be worth mentioning in the proposal.

@jsumners
Copy link

I believe there's another reason to stick to error first in js, and that's for language consistency in Nodejs, the most used JS server runtime.

Error first for callbacks makes sense because you always want to check the error, and parameters after the first one are not guaranteed.

Error first in the proposal is very odd, in particular because the pattern it is emulating doesn't do error first. So it will be confusing for people working in multiple languages. Thus, it will be a source of friction. But it's also weird without considering any other languages:

// As proposed:
const [error, data] = await something()
// But if we insist on being silly and ignoring the error:
const [ , data] = await something()

// Switching to error second:
const [data, error] = await something()
// And that silly case again:
const [data] = await something()

In other words, as proposed, I can envision code bases littered with ugle [, data] all over the place.

@arthurfiorette
Copy link
Owner

arthurfiorette commented Aug 16, 2024

Having [data, error], or [data] we can easily SUPRESS the error which is different than just ignoring the error.

Ignoring an error:

const val = await fn()
// this throws if fn rejects

Suppressing the error:

const [, data] = await fn()
const val = fn().catch(() => null)
// this simply does nothing and makes val undefined if fn throws.

From the What This Proposal Does Not Aim to Solve section of this proposal:

Automatic Error Handling: While this proposal facilitates error handling, it does not automatically handle errors for you. You will still need to write the necessary code to manage errors; the proposal simply aims to make this process easier and more consistent.

Completely suppressing errors is totally different from just ignoring the possibility of a rejection and the handle needed in case it rejects.

// this gets data in a simpler syntax and throws if an error happens
const data = fn()

// This is WORSE because it simply makes the error disappear.
const [data] ?= fn()

And forgetting to add a , error in the tuple destructuring is something we might mistakenly do. This proposal does not want to increase the number of silly mistakes because of bad syntax.


Besides that, it also follows an already present syntax in callbacks, where the error parameter is always the first one.

@jsumners
Copy link

Besides that, it also follows an already present syntax in callbacks, where the error parameter is always the first one.

I addressed that at the top of my comment.

[,data] is not as legible as [data].

@Wellbrito29
Copy link

Besides that, it also follows an already present syntax in callbacks, where the error parameter is always the first one.

I addressed that at the top of my comment.

[,data] is not as legible as [data].

why not legible?

@leppaott
Copy link

Besides that, it also follows an already present syntax in callbacks, where the error parameter is always the first one.

I addressed that at the top of my comment.

[,data] is not as legible as [data].

This, but isn't it better to make ignoring the error harder? If you deliberately want to ignore it, do not use this operator or maybe not babysit and have data as first? I had the same thought too, somehow seemed clearer and could just follow Go to make jumping languages easier. Why make superficial changes without much point? Just keep it same... Eslint can have a rule to warn about not handling error or otherwise it's a mistake to use this operator. No point to reason adjusting something avoids mistakes.

@adrianulima
Copy link

IMHO data-first approach is simply better for both readability and usage. The ability to ignore an error could be a valuable feature.
Checking if an error is not null is essentially the same as checking if a result is not undefined. In some cases, that is all we want (no error handling needed).
I believe this decision could overshadow the entire proposal, mostly because of an ideology (enforcing error handling over better usability).
But, this is just my opinion. I liked the rest of the proposal.

@andydavies-cupa
Copy link

The [,data] syntax feels implicit and clumsy to me as it could result from a user error. I would rather be in favor something explicit, so that someone else reading the code knows the intention is to ignore the error. C# has the concept of discards which if was copied would then give:

[ _, data]

To me the intention of the code is therefore clear, the original author is making the choice to discard the not use the first value.

@arthurfiorette
Copy link
Owner

[ _, data]

To me the intention of the code is therefore clear, the original author is making the choice to discard the not use the first value.

Makes perfect sense, I'll try to add this on the spec, although i didn't want to facilitate suppressing errors in any way, but since this proposal uses errors as values, i guess there's nowhere else to go.

@arthurfiorette
Copy link
Owner

In other words, as proposed, I can envision code bases littered with ugle [, data] all over the place.

I can't, since doing [_, data] or [data] would make the data T | undefined and you would be forced to check if data is defined either way. If you don't want to handle errors, just const data = fn() and forget about it. If you know that the function will throw an error and wants to ignore it either way, ?= is not the solution because using it would still force you to check if data is defined or not.

@arthurfiorette
Copy link
Owner

Also, for functions that returns void but might throw an error:

const [error] ?= await sendDataToServer(data);

if (error) {
  console.warn('oops')
}

// success!

@arthurfiorette arthurfiorette added the documentation Improvements or additions to documentation label Aug 21, 2024
@jsumners
Copy link

Also, for functions that returns void but might throw an error:

const [error] ?= await sendDataToServer(data);



if (error) {

  console.warn('oops')

}



// success!

This is a fair argument.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

7 participants