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

Reduce without an initialValue should be illegal on a potentially empty array #34736

Closed
calimeroteknik opened this issue Oct 25, 2019 · 9 comments
Labels
Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds

Comments

@calimeroteknik
Copy link

TypeScript Version: 3.8.0-dev.20191025

Search Terms: reduce no initialvalue

Code

[].reduce((a,b) => a+b /*, initialvalue NOT specified*/);

Expected behavior:
Should not compile

Actual behavior:
Compiles and crashes:

~ $ tsc reduce.ts; echo $?
0
~ $ js reduce.js
reduce.js:1: TypeError: reduce of empty array with no initial value

Related Issues:
None relevant, #28901 is trying to make TS accept something that it doesn't accept currently; I'm asking the opposite: that something which is accepted became not accepted.

Some ideas:
Retiring reduce with no initialValue from the type Array as this is unsafe to keep around; people should provide an initialValue if their array can be empty:

--- a/typescript/lib/lib.es5.d.ts	2019-10-25 12:33:06.312693040 +0200
+++ b/typescript/lib/lib.es5.d.ts	2019-10-25 12:34:11.071987865 +0200
@@ -1329,6 +1329,7 @@
       * @param callbackfn A function that accepts up to four arguments. The reduce method calls the callbackfn function one time for each element in the array.
       * @param initialValue If initialValue is specified, it is used as the initial value to start the accumulation. The first call to the callbackfn function provides this value as an argument instead of an array value.
       */
-    reduce(callbackfn: (previousValue: T, currentValue: T, currentIndex: number, array: T[]) => T): T;
     reduce(callbackfn: (previousValue: T, currentValue: T, currentIndex: number, array: T[]) => T, initialValue: T): T;
     /**
       * Calls the specified callback function for all the elements in an array. The return value of the callback function is the accumulated result, and is provided as an argument in the next call to the callback function.

In order to use reduce without initialValue on arrays for which it will work, it's possible to introduce something that might or might not be along the lines of:

type NonEmptyArray<T> = [T, ...T[]];

This non-empty array type can safely reintroduce the reduce signature without initialValue, the very one I would like to retire from the (possibly empty) Array type.

People who want to live dangerously could still use ([] as NonEmptyArray).reduce(someOperation) if they absolutely refuse to provide an initialValue.

@AnyhowStep
Copy link
Contributor

AnyhowStep commented Oct 25, 2019

How about we just delete the .reduce() signature without an initialValue entirely? =P

It's rare that one uses the .reduce() method and performs an operation that does not have an identity element.

For example,

//Zero is the identity element of addition
arr.reduce((result, item) => result + item, 0);

//One is the identity element of multiplication
arr.reduce((result, item) => result * item, 1);

//Empty object is the identity element of adding properties to stuff
arr.reduce((result, item) => { result[item] = 0; return result; }, {} as Record<string, unknown>);

I'm just kidding about removing the signature entirely, by the way.

Of course, there are moments where there is no identity element,

arr.reduce((result, item) => result / item, /*no identity element*/);

@calimeroteknik
Copy link
Author

calimeroteknik commented Oct 25, 2019

In fact, there are no moments where there is no identity element, I wager. It would rather be the developer exploiting the mathematically clunky version of reduce (the one where you give no identity element that is).

To demonstrate, the above example,

arr.reduce((result, item) => result / item, /*no identity element*/);

…translates to English as "the first element of the list, divided by all others in turn", a sentence where I notice the part "the first element of the list", and which therefore leaves a gaping hole in the definition for the case of empty lists: it is not defined on them.

Emphasis on the fact that the first element is treated very differently from the rest, which should somehow be visible in the implementation. I believe the example is therefore more mathematically put as:

arr.slice(1).reduce((result, item) => result / item, arr[0]);

And the reference to the first element makes it clear as day that there will be a problem if we have nothing in the list.
…Although as it turns out, even tsc --strict doesn't complain about it either and we get undefined. Well, technically, indeed. But, augh!

I believe that TS should only allow static references, e.g. arr[2] to array indexes that provably exist.
With the method mentioned earlier:

let foo: [T, T, T ...T[]] = [9, 12, 4, 5, 14, 7];
foo[0], foo[1]; // ok
foo[2]; // ok
foo[3]; // error

I explore this because the topic of arrays with some guarantees on their length isn't irrelevant to in-depth handling of the present issue (which I'd very much like to explore solutions for).

@AnyhowStep
Copy link
Contributor

AnyhowStep commented Oct 25, 2019

Would be cool to have type guards like,

declare const arr : T[];
if (arr.length > 0) {
  //arr is now `[T, ...T[]]`
}
if (arr.length >= 2) {
  //arr is now `[T, T, ...T[]]`
}
if (arr.length == 4) {
  //arr is now `[T, T, T, T]`
}

This way, the overload for .reduce(this : [T, ...T[]], reducer) would be way easier to use.


I think there's a proposal for such a type guard... I'm not sure.

[EDIT]

Found it

#28837

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Oct 30, 2019
@RyanCavanaugh
Copy link
Member

This level of enforcement, while coherent, would be far far above anything currently on the table, to say nothing of consistency -- even today arr[0] is allowed regardless of type information!

Many times arrays are known to be non-empty by construction and this would introduce a lot of unneeded friction.

@calimeroteknik
Copy link
Author

calimeroteknik commented Oct 31, 2019

There are two sides to this indeed:

  • on the one hand, this does go farther than the current set of checks;
  • on the other hand, to typecheck basic usage of JS built-ins correctly, one has to go this far.

I suggest this rule of thumb: when the JS engine throws a TypeError at run time, something is wrong with the static typing.

--

Concerning the "unneeded" friction (thinking of existing code, I assume), I think that is an argument for the part of the debate about whether to have array length checks as a default.

I am aware that people who care less and do not actually rely on the type system, considering it a bonus, will feel it's a bother if they need to satisfy the type checker or forcefully type-assert that they know what they are doing.

However, people who use --strict typically seek and expect pedantic behaviour from the compiler, and feel betrayed when type checking overlooks simple and detectable, yet deadly mistakes:

[…] even made me lose trust in the type system […] #28682

People who decide to use a type system do it specifically because they find friction far more desirable than potential run time errors.

[edit] What I am questioning is whether this could indeed be "Working as Intended", not the fact that this can be classified as an improvement request rather than a bug report.

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds and removed Working as Intended The behavior described is the intended behavior; this is not a bug labels Oct 31, 2019
@RyanCavanaugh
Copy link
Member

I've changed the labels - let me know if there are others you think would be better

@typescript-bot
Copy link
Collaborator

This issue has been marked as "Too Complex" and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@typescript-bot typescript-bot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 21, 2023
@voliva
Copy link

voliva commented Jun 30, 2023

In order to use reduce without initialValue on arrays for which it will work, it's possible to introduce something that might or might not be along the lines of:

type NonEmptyArray<T> = [T, ...T[]];

This non-empty array type can safely reintroduce the reduce signature without initialValue, the very one I would like to retire from the (possibly empty) Array type.

This wouldn't actually work.... How can you protect from:

// Type NonEmptyArray<T>
const myNonEmptyArray = [1,2,3];

myNonEmptyArray.reduce((a, b) => a+b) // Yay, all good

while(myNonEmptyArray.length)
  myNonEmptyArray.pop();

myNonEmptyArray.reduce((a, b) => a+b) // TypeError on runtime

This is why I'm also on the side that trying to do something smart will always add complexity and friction, and even probably not solve the problem it was trying to solve initially.

@calimeroteknik
Copy link
Author

calimeroteknik commented Jul 1, 2023

@voliva I expect an error here:

while(myNonEmptyArray.length)
  myNonEmptyArray.pop();

Because you can't empty a non-empty (aka non-emptiable) array!

I'd like to send you back to #34736 (comment) in this thread and the proposal mentioned, #28837

Alternatively, its type was mutated (the opposite of a guard) when emptying it, and things work again, there's a type error on the reduce invocation. But that's too far-fetched for me (specifically, I'm not convinced about mutating types).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds
Projects
None yet
Development

No branches or pull requests

5 participants