Stop turf-mask mutating by default, make it an option (#2635)

* Add a new test for the behavior we want

* Add mutate option & default to cloning the mask input unless mutate is
set to true.

Also, add another test for this.

* update docs

* fix pnpm-lock.yaml

* Update benchmark to include new `mutate` option

* cleanup

* Bringing recent TS version of mask into this PR. Should hopefully minimise conflicts required to be resolved before merging.

* Missed adding @turf/clone to the prod dependencies.

* Add new arg to type tests

* resolve TypeScript error in bench.ts

* Remove benchmark skipped input

multi-polygon.geojson
overlapping.geojson

---------

Co-authored-by: James Beard <james@smallsaucepan.com>
Co-authored-by: mfedderly <24275386+mfedderly@users.noreply.github.com>

packages/turf-mask/README.md

@@ -11,6 +11,9 @@ ring polygon with holes.
 
 *   `polygon` **([Polygon][1] | [MultiPolygon][2] | [Feature][3]<([Polygon][1] | [MultiPolygon][2])> | [FeatureCollection][4]<([Polygon][1] | [MultiPolygon][2])>)** GeoJSON polygon used as interior rings or holes
 *   `mask` **([Polygon][1] | [Feature][3]<[Polygon][1]>)?** GeoJSON polygon used as the exterior ring (if undefined, the world extent is used)
+*   `options` **[Object][5]** Optional parameters (optional, default `{}`)
+
+    *   `options.mutate` **[boolean][6]** allows the `mask` GeoJSON input to be mutated (performance improvement if true) (optional, default `false`)
 
 ### Examples
 
@@ -24,7 +27,7 @@ const masked = turf.mask(polygon, mask);
 const addToMap = [masked]
 ```
 
-Returns **[Feature][3]<[Polygon][1]>** Masked Polygon (exterior ring with holes).
+Returns **[Feature][3]<[Polygon][1]>** Masked Polygon (exterior ring with holes)
 
 [1]: https://tools.ietf.org/html/rfc7946#section-3.1.6
 
@@ -34,6 +37,10 @@ Returns **[Feature][3]<[Polygon][1]>** Masked Polygon (exterior ring with holes)
 
 [4]: https://tools.ietf.org/html/rfc7946#section-3.3
 
+[5]: https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Object
+
+[6]: https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Boolean
+
 <!-- This file is automatically generated. Please don't edit it directly. If you find an error, edit the source file of the module in question (likely index.js or index.ts), and re-run "yarn docs" from the root of the turf project. -->
 
 ---

packages/turf-mask/bench.ts

@@ -1,13 +1,21 @@
-import { Feature, FeatureCollection, Polygon, MultiPolygon } from "geojson";
+import { FeatureCollection, Polygon, MultiPolygon, Feature } from "geojson";
 import fs from "fs";
 import path from "path";
 import { fileURLToPath } from "url";
 import { loadJsonFileSync } from "load-json-file";
 import Benchmark, { Event } from "benchmark";
 import { mask as turfMask } from "./index.js";
+import clone from "@turf/clone";
+
+// type guard to narrow the type of the fixtures
+const isPolygonFeature = (
+  feature: Feature<Polygon | MultiPolygon>
+): feature is Feature<Polygon> => feature.geometry.type === "Polygon";
 
 const __dirname = path.dirname(fileURLToPath(import.meta.url));
 
+const SKIP = [];
+
 const suite = new Benchmark.Suite("turf-mask");
 
 const directories = {
@@ -17,22 +25,44 @@ const directories = {
 
 let fixtures = fs.readdirSync(directories.in).map((filename) => {
   return {
+    filename,
     name: path.parse(filename).name,
     geojson: loadJsonFileSync(
       path.join(directories.in, filename)
     ) as FeatureCollection<Polygon | MultiPolygon>,
   };
 });
 
-for (const { name, geojson } of fixtures) {
+for (const { name, filename, geojson } of fixtures) {
+  if (SKIP.includes(filename)) continue;
+
   const [polygon, masking] = geojson.features;
-  suite.add(name, () => turfMask(polygon, masking as Feature<Polygon>));
+  if (!masking || !isPolygonFeature(masking)) {
+    throw new Error(
+      "Fixtures should have two features: an input feature and a Polygon masking feature."
+    );
+  }
+
+  const getSuite = ({ mutate }: { mutate: boolean }) => ({
+    name: `${name} (mutate = ${mutate})`,
+    fn: () => {
+      // We clone the inputs to prevent tests from affecting each other
+      turfMask(clone(polygon), clone(masking), { mutate });
+    },
+  });
+
+  suite.add(getSuite({ mutate: false }));
+  suite.add(getSuite({ mutate: true }));
 }
 
-// basic x 4,627 ops/sec ±25.23% (21 runs sampled)
-// mask-outside x 3,892 ops/sec ±34.80% (15 runs sampled)
-// multi-polygon x 5,837 ops/sec ±3.03% (91 runs sampled)
-// overlapping x 22,326 ops/sec ±1.34% (90 runs sampled)
+/**
+ * Benchmark Results:
+ *
+ * basic (mutate = false) x 294,373 ops/sec ±0.25% (95 runs sampled)
+ * basic (mutate = true) x 307,397 ops/sec ±0.13% (97 runs sampled)
+ * mask-outside (mutate = false) x 100,575 ops/sec ±0.55% (97 runs sampled)
+ * mask-outside (mutate = true) x 103,180 ops/sec ±0.40% (94 runs sampled)
+ */
 suite
   .on("cycle", (event: Event) => {
     console.log(String(event.target));

packages/turf-mask/index.ts

@@ -7,6 +7,7 @@ import {
 } from "geojson";
 import { polygon as createPolygon, multiPolygon } from "@turf/helpers";
 import polygonClipping, { Geom } from "polygon-clipping";
+import { clone } from "@turf/clone";
 
 /**
  * Takes polygons or multipolygons and an optional mask, and returns an exterior
@@ -15,7 +16,9 @@ import polygonClipping, { Geom } from "polygon-clipping";
  * @name mask
  * @param {Polygon|MultiPolygon|Feature<Polygon|MultiPolygon>|FeatureCollection<Polygon|MultiPolygon>} polygon GeoJSON polygon used as interior rings or holes
  * @param {Polygon|Feature<Polygon>} [mask] GeoJSON polygon used as the exterior ring (if undefined, the world extent is used)
- * @returns {Feature<Polygon>} Masked Polygon (exterior ring with holes).
+ * @param {Object} [options={}] Optional parameters
+ * @param {boolean} [options.mutate=false] allows the `mask` GeoJSON input to be mutated (performance improvement if true)
+ * @returns {Feature<Polygon>} Masked Polygon (exterior ring with holes)
  * @example
  * const polygon = turf.polygon([[[112, -21], [116, -36], [146, -39], [153, -24], [133, -10], [112, -21]]]);
  * const mask = turf.polygon([[[90, -55], [170, -55], [170, 10], [90, 10], [90, -55]]]);
@@ -27,10 +30,19 @@ import polygonClipping, { Geom } from "polygon-clipping";
  */
 function mask<T extends Polygon | MultiPolygon>(
   polygon: T | Feature<T> | FeatureCollection<T>,
-  mask?: Polygon | Feature<Polygon>
+  mask?: Polygon | Feature<Polygon>,
+  options?: { mutate?: boolean }
 ): Feature<Polygon> {
-  // Define mask
-  const maskPolygon = createMask(mask);
+  const mutate = options?.mutate ?? false; // by default, do not mutate
+
+  let maskTemplate = mask;
+  if (mask && mutate === false) {
+    // Clone mask if requested to avoid side effects
+    maskTemplate = clone(mask);
+  }
+
+  // Define initial mask
+  const maskPolygon = createMask(maskTemplate);
 
   let polygonOuters = null;
   if (polygon.type === "FeatureCollection") {

packages/turf-mask/package.json

@@ -64,6 +64,7 @@
     "write-json-file": "^5.0.0"
   },
   "dependencies": {
+    "@turf/clone": "workspace:^",
     "@turf/helpers": "workspace:^",
     "@types/geojson": "^7946.0.10",
     "polygon-clipping": "^0.15.3",

packages/turf-mask/test.ts

@@ -12,6 +12,7 @@ import { fileURLToPath } from "url";
 import { loadJsonFileSync } from "load-json-file";
 import { writeJsonFileSync } from "write-json-file";
 import { mask } from "./index.js";
+import { clone } from "@turf/clone";
 
 const __dirname = path.dirname(fileURLToPath(import.meta.url));
 
@@ -48,6 +49,36 @@ test("turf-mask", (t) => {
   t.end();
 });
 
+const getBasicPolygonAndMask = () => {
+  const basicFixture = fixtures.find(
+    ({ filename }) => filename === "basic.geojson"
+  );
+  if (!basicFixture) throw new Error("basic.geojson not found");
+  return basicFixture.geojson.features;
+};
+
+test("turf-mask -- doesn't mutate inputs by default", (t) => {
+  const [polygon, masking] = getBasicPolygonAndMask();
+  const maskClone = clone(masking);
+
+  mask(polygon, masking);
+
+  t.deepEquals(masking, maskClone, "mask input should not be mutated");
+
+  t.end();
+});
+
+test("turf-mask -- mutates mask input when mutate = true", (t) => {
+  const [polygon, masking] = getBasicPolygonAndMask();
+  const maskClone = clone(masking);
+
+  mask(polygon, masking, { mutate: true });
+
+  t.notDeepEqual(masking, maskClone, "mask input should be mutated");
+
+  t.end();
+});
+
 test("turf-mask polygon geometry", (t) => {
   // A polygon somewhere
   const polyCoords: Position[] = [

packages/turf-mask/types.ts

@@ -22,3 +22,4 @@ const poly2 = polygon([
 
 mask(poly1);
 mask(poly1, poly2);
+mask(poly1, poly2, { mutate: true });