Define/follow return types conventions

[MisRob]

Observed behavior

We've started to use a new lint rule that enforces return statement being present in computed properties but we have no convention on return types which causes inconsistency. Also it's not clear which types are considered to be valid return types and which are considered rather unexpected.

Expected behavior

There is a set of return types rules defined (the first discussion can be found here #5331). If possible, there's a lint rule enforcing our new conventions.

User-facing consequences

None

[jonboiser]

Here's a process we can do for each component that has a method or computed property that returns undefined after #5331

1. If the function returns the union type A | undefined , change it so it only returns A by replacing the returned undefined with the "neutral" value for type A (e.g. '' for String or [] for Array, etc.).
2. Look at all dependents of the function.
3. If the the function's dependents work the same with neutral value, then you're done.
4. If the function's dependents don't work anymore when passed the neutral value, then reimplement the function to return null in the exceptional case, giving it a type of Maybe A = A | null.
5. In the function's dependents, branch its logic to handle the generate null with an explicit guard (if x === null...) that does not do any type conversion so that it works as before.

[bjester]

I caught sight of the PR and just wanted to throw in my own thoughts in case they may be helpful.

If the function returns the union type A | undefined, change it so it only returns A by replacing the returned undefined with the "neutral" value for type A (e.g. '' for String or [] for Array, etc.).

An empty array could be a meaningful result, so replacing what would be a missing return, return undefined or return null with return [] doesn't convey the same thing, in my opinion. I think it's similar for strings and others.

An example that comes to mind to explain this would be an asynchronous function that returns a Promise. If it's calling an API to get a list, and it doesn't find anything, it would make sense to resolve with []. Otherwise, if it encountered an unknown scenario, it should reject the promise. Of course, if the rejected promise is unhandled, it will throw an error.

Similarly, in a synchronous function, I think null represents this rejected/unexpected state. If it is not handled by the caller, then an error would/should be thrown (ie .forEach on null). Otherwise, the caller can handle the null result as something special or simply coalesce it away. Uncaught unexpected results can be useful because they throw errors and draw attention to the scenario that wasn't expected. If they don't, yay -- nothing unexpected!

Again, just my thoughts!

[jonboiser]

I think this is could be up to personal preference, especially since we're discussing usage within Vue components. I think for general functions it might make sense to pass on failures/nulls (although sometimes it's nice when functions handled failures gracefully).

But within a component, if you have a computed property called transformedData, and this template:

<div v-for="x in transformedArray"> {{ x.name }}</div>

You would actually get the same result if transformedArray were [] or null or undefined.

But for the sake of readability, it's sometimes nice to look at the implementation transformedArray and it's clear that it's intended to be an array no matter which branch you're scanning.

[MisRob]

I think this is could be up to personal preference, especially since we're discussing usage within Vue components.

Although Vue components were the first reason for this discussion, would it make sense to define how it's supposed to work for all functions in general, not just for Vue components computed and methods? Also, the discussion in #5331 was general enough I think. Sometimes you can actually use various utils or API calls within Vue components so I suppose it should play well together as well? And it might be easier for everyone to remember one simple general convention.

I was thinking that at first, we could define the convention as discussed in #5331 (in terms of this, I agree with @indirectlylit proposal) and then, based on the final decision, mentioned computed/methods from #5331 can be updated to follow it and it can be used for all new features (with the possibility of update of old codes, not sure about the strategy yet..)? Does it make sense to you guys?

[bjester]

And it might be easier for everyone to remember one simple general convention.

That's a good point!

Also, returning null gives you the ability to be as explicit or implicit as you want in the template, which in my experience is great.

@jonboiser In your example, I think allowing null to be implicitly handled as [] is just fine, but if you wanted to be explicit, you would still have the option to do so if null was returned:

<div v-for="x in transformedArray | coalesce []"> {{ x.name }}</div>

given a defined coalesce filter, or it can be handled as a special case in the template:

<div v-if="transformedArray !== null">
  <div v-for="x in transformedArray"> {{ x.name }}</div>
</div>
<div v-else>Strange</div>