Merge pull request #2331 from akiran/fix-2315

Fixed #2315

Author: akiran

.prettierrc

@@ -0,0 +1 @@
+trailingComma: none

__tests__/regression/fix-2315.test.js

@@ -0,0 +1,35 @@
+// Test fix of #2315: Slider crashing after a state change in parent component
+import React from "react";
+import { render, fireEvent } from "@testing-library/react";
+
+import { getCurrentSlideContent, getSlidesCount } from "../../test-utils";
+import { GenericSliderComponent } from "../TestComponents";
+
+function TestSlider() {
+  const [count, setCount] = React.useState();
+
+  return (
+    <div>
+      <button className="increment-button" onClick={() => setCount(count + 1)}>
+        Increment {count}
+      </button>
+      <GenericSliderComponent slidesCount={6} settings={{ count }} />
+    </div>
+  );
+}
+
+describe("State change in parent component of slider", () => {
+  it("Slider shoud work afer clicking on Increment button", function() {
+    const { container } = render(<TestSlider />);
+    fireEvent(
+      container.getElementsByClassName("increment-button")[0],
+      new MouseEvent("click", {
+        bubbles: true,
+        cancelable: true
+      })
+    );
+    // Throws an error "Maximum update depth exceeded." if the bug exists
+    expect(getCurrentSlideContent(container)).toEqual("1");
+    expect(getSlidesCount(container)).toEqual(13);
+  });
+});

__tests__/utils/filterSettings.test.js

@@ -0,0 +1,18 @@
+import { filterSettings } from "../../src/utils/innerSliderUtils";
+
+describe("filterSettings", () => {
+  it("returns empty object if there are no valid settings", () => {
+    expect(filterSettings({})).toEqual({});
+    expect(filterSettings({ age: 10 })).toEqual({});
+  });
+  it("return an object with valid settings and omits extra properties", () => {
+    expect(filterSettings({ arrows: true, dots: true })).toEqual({
+      arrows: true,
+      dots: true
+    });
+    expect(filterSettings({ arrows: true, dots: true, age: 10 })).toEqual({
+      arrows: true,
+      dots: true
+    });
+  });
+});

package.json

@@ -18,7 +18,8 @@
     "gen": "node examples/scripts/generateExampleConfigs.js && node examples/scripts/generateExamples.js && xdg-open docs/jquery.html",
     "precommit": "lint-staged",
     "test": "jest",
-    "test-watch": "jest --watch"
+    "test-watch": "jest --watch",
+    "clear-jest": "jest --clearCache"
   },
   "author": "Kiran Abburi",
   "license": "MIT",

src/inner-slider.js

@@ -134,7 +134,8 @@ export class InnerSlider extends React.Component {
       }
       if (
         typeof prevProps[key] === "object" ||
-        typeof prevProps[key] === "function"
+        typeof prevProps[key] === "function" ||
+        isNaN(prevProps[key])
       ) {
         continue;
       }

src/slider.js

@@ -4,7 +4,7 @@ import React from "react";
 import { InnerSlider } from "./inner-slider";
 import json2mq from "json2mq";
 import defaultProps from "./default-props";
-import { canUseDOM } from "./utils/innerSliderUtils";
+import { canUseDOM, filterSettings } from "./utils/innerSliderUtils";
 const enquire = canUseDOM() && require("enquire.js");
 
 export default class Slider extends React.Component {
@@ -198,14 +198,17 @@ export default class Slider extends React.Component {
     if (settings === "unslick") {
       const className = "regular slider " + (this.props.className || "");
       return <div className={className}>{children}</div>;
-    } else if (newChildren.length <= settings.slidesToShow && !settings.infinite) {
+    } else if (
+      newChildren.length <= settings.slidesToShow &&
+      !settings.infinite
+    ) {
       settings.unslick = true;
     }
     return (
       <InnerSlider
         style={this.props.style}
         ref={this.innerSliderRefHandler}
-        {...settings}
+        {...filterSettings(settings)}
       >
         {newChildren}
       </InnerSlider>

src/utils/innerSliderUtils.js

@@ -1,15 +1,16 @@
 import React from "react";
+import defaultProps from "../default-props";
 
 export function clamp(number, lowerBound, upperBound) {
   return Math.max(lowerBound, Math.min(number, upperBound));
 }
 
 export const safePreventDefault = event => {
   const passiveEvents = ["onTouchStart", "onTouchMove", "onWheel"];
-  if(!passiveEvents.includes(event._reactName)) {
+  if (!passiveEvents.includes(event._reactName)) {
     event.preventDefault();
   }
-}
+};
 
 export const getOnDemandLazySlides = spec => {
   let onDemandSlides = [];
@@ -386,9 +387,12 @@ export const swipeMove = (e, spec) => {
   let touchSwipeLength = touchObject.swipeLength;
   if (!infinite) {
     if (
-      (currentSlide === 0 && (swipeDirection === "right" || swipeDirection === "down")) ||
-      (currentSlide + 1 >= dotCount && (swipeDirection === "left" || swipeDirection === "up")) ||
-      (!canGoNext(spec) && (swipeDirection === "left" || swipeDirection === "up"))
+      (currentSlide === 0 &&
+        (swipeDirection === "right" || swipeDirection === "down")) ||
+      (currentSlide + 1 >= dotCount &&
+        (swipeDirection === "left" || swipeDirection === "up")) ||
+      (!canGoNext(spec) &&
+        (swipeDirection === "left" || swipeDirection === "up"))
     ) {
       touchSwipeLength = touchObject.swipeLength * edgeFriction;
       if (edgeDragged === false && onEdge) {
@@ -849,3 +853,14 @@ export const canUseDOM = () =>
     window.document &&
     window.document.createElement
   );
+
+export const validSettings = Object.keys(defaultProps);
+
+export function filterSettings(settings) {
+  return validSettings.reduce((acc, settingName) => {
+    if (settings.hasOwnProperty(settingName)) {
+      acc[settingName] = settings[settingName];
+    }
+    return acc;
+  }, {});
+}