From 2a51e602c067f1e2f9387600307544b97b02a239 Mon Sep 17 00:00:00 2001
From: Pete Murphy <26548438+pete-murphy@users.noreply.github.com>
Date: Sat, 15 Feb 2025 13:25:08 -0500
Subject: [PATCH] Avoid `RangeError` in `arrayBind` foreign implementation
 (#314)

* test(#309): Failing test

Demonstrating that the current implementation of `Array`'s `Bind` instance
causes `RangeError: Maximum call stack size exceeded` when the output of `f` in
`ma >>= f` is sufficiently large.

This is due to usage of `Function.prototype.apply`. From
[MDN](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/apply#using_apply_and_built-in_functions):

> But beware: by using apply() (or the spread syntax) with an arbitrarily long
arguments list, you run the risk of exceeding the JavaScript engine's argument
length limit.

> The consequences of calling a function with too many arguments (that is, more
than tens of thousands of arguments) is unspecified and varies across engines.
(The JavaScriptCore engine has a hard-coded [argument limit of
65536](https://webkit.org/b/80797).)

Node v20.18.1 seems to have a higher limit around 106,000.

* fix(#309): Use `flatMap` if supported by runtime

* fix(#309): Use simple stack-safe fallback

* chore(#309): Add to CHANGELOG.md

* feat(#309): Address feedback from code review

Using static check to determine if `Array.prototype.flatMap` is available, and
use `var` instead of `let` in for loop to match existing code style.

---------

Co-authored-by: Peter Murphy <26548438+ptrfrncsmrph@users.noreply.github.com>
---
 CHANGELOG.md        |  1 +
 src/Control/Bind.js | 28 ++++++++++++++++++++--------
 test/Test/Main.js   |  8 ++++++++
 test/Test/Main.purs | 12 ++++++++++++
 4 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index d7e5af83..5f2f4d42 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,6 +9,7 @@ Breaking changes:
 New features:
 
 Bugfixes:
+- Avoid `RangeError` in `arrayBind` foreign implementation (#314 by @pete-murphy)
 
 Other improvements:
 
diff --git a/src/Control/Bind.js b/src/Control/Bind.js
index fa0dbaeb..bf79719f 100644
--- a/src/Control/Bind.js
+++ b/src/Control/Bind.js
@@ -1,9 +1,21 @@
-export const arrayBind = function (arr) {
-  return function (f) {
-    var result = [];
-    for (var i = 0, l = arr.length; i < l; i++) {
-      Array.prototype.push.apply(result, f(arr[i]));
+export const arrayBind =
+  typeof Array.prototype.flatMap === "function"
+    ? function (arr) {
+      return function (f) {
+        return arr.flatMap(f);
+      };
     }
-    return result;
-  };
-};
+    : function (arr) {
+      return function (f) {
+        var result = [];
+        var l = arr.length;
+        for (var i = 0; i < l; i++) {
+          var xs = f(arr[i]);
+          var k = xs.length;
+          for (var j = 0; j < k; j++) {
+            result.push(xs[j]);
+          }
+        }
+        return result;
+      };
+    };
diff --git a/test/Test/Main.js b/test/Test/Main.js
index b25cdaca..72ebcd92 100644
--- a/test/Test/Main.js
+++ b/test/Test/Main.js
@@ -40,3 +40,11 @@ export function testNumberShow(showNumber) {
     ]);
   };
 }
+
+export function makeArray(length) {
+  var arr = [];
+  for (var i = 0; i < length; i++) {
+    arr.push(i);
+  }
+  return arr;
+}
diff --git a/test/Test/Main.purs b/test/Test/Main.purs
index 13ea2cce..fd763cf0 100644
--- a/test/Test/Main.purs
+++ b/test/Test/Main.purs
@@ -22,6 +22,7 @@ main = do
     testReflectType
     testReifyType
     testSignum
+    testArrayBind
 
 foreign import testNumberShow :: (Number -> String) -> AlmostEff
 
@@ -189,3 +190,14 @@ testSignum = do
   assert "signum positive zero" $ show (1.0/(signum 0.0)) == "Infinity"
   assert "Clarifies what 'signum negative zero' test is doing" $ show (1.0/(-0.0)) == "-Infinity"
   assert "signum negative zero" $ show (1.0/(signum (-0.0))) == "-Infinity"
+
+foreign import makeArray :: Int -> Array Int
+
+testArrayBind :: AlmostEff
+testArrayBind = do
+  assert "Array bind does not cause RangeError" do
+    let
+      _ = do
+        _ <- [unit]
+        makeArray 106_000
+    true