fix: re-render Background only if a larger image is needed (#782)

* fix(BackgroundImpl): re-render only if a larger image is needed * refactor: component should update if props or imgixParams change * chore(release): 9.1.0 ## [9.1.0](v9.0.4...v9.1.0) (2021-04-22) ### Features * integrate @imgix/js-core into react-imgix ([#780](#780)) ([690e7b6](690e7b6)), closes [#763](#763) [skip ci] Co-authored-by: imgix-git-robot <support@imgix.com>
imgix · Apr 23, 2021 · 53bb104 · 53bb104
1 parent 0fe4d12
commit 53bb104
Show file tree

Hide file tree

Showing 3 changed files with 191 additions and 90 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,5 @@
-## [9.1.0](https://github.com/imgix/react-imgix/compare/v9.0.4...v9.1.0) (2021-04-23)
 
+## [9.1.0](https://github.com/imgix/react-imgix/compare/v9.0.4...v9.1.0) (2021-04-23)
 
 ### Features
 

diff --git a/src/react-imgix-bg.jsx b/src/react-imgix-bg.jsx
@@ -5,6 +5,7 @@ import constructUrl from "./constructUrl";
 import extractQueryParams from "./extractQueryParams";
 import findClosest from "./findClosest";
 import targetWidths from "./targetWidths";
+import { shallowEqual } from "./common";
 
 const noop = () => {};
 
@@ -13,107 +14,153 @@ const findNearestWidth = (actualWidth) =>
 
 const toFixed = (dp, value) => +value.toFixed(dp);
 
-const BackgroundImpl = (props) => {
-  const {
-    measureRef,
-    measure,
-    contentRect,
-    imgixParams = {},
-    onLoad,
-    disableLibraryParam,
-    src,
-    children,
-    className = "",
-  } = props;
-  const { w: forcedWidth, h: forcedHeight } = imgixParams;
-  const hasDOMDimensions =
-    contentRect.bounds.width != null && contentRect.bounds.height != null;
-  const htmlAttributes = props.htmlAttributes || {};
-  const dpr = toFixed(2, imgixParams.dpr || global.devicePixelRatio || 1);
-  const ref = htmlAttributes.ref;
-  const onRef = (el) => {
-    measureRef(el);
-    if (typeof ref === "function") {
-      ref(el);
-    }
-  };
+class BackgroundImpl extends React.Component {
+  constructor(props) {
+    super(props);
+  }
+
+  shouldComponentUpdate(nextProps) {
+    const contentRect = this.props.contentRect;
+    const bounds = contentRect.bounds;
+    const { width: prevWidth, height: prevHeight } = bounds;
 
-  const { width, height } = (() => {
-    const bothWidthAndHeightPassed =
-      forcedWidth != null && forcedHeight != null;
+    const nextContentRect = nextProps.contentRect;
+    const nextBounds = nextContentRect.bounds;
+    const { width: nextWidth, height: nextHeight } = nextBounds;
 
-    if (bothWidthAndHeightPassed) {
-      return { width: forcedWidth, height: forcedHeight };
+    // If neither of the previous nor next dimensions are present,
+    // re-render.
+    if (!nextWidth || !nextHeight || !prevWidth || !prevHeight) {
+      return true;
     }
 
-    if (!hasDOMDimensions) {
-      return { width: undefined, height: undefined };
+    // The component has been rendered at least twice by this point
+    // and both the previous and next dimensions should be defined.
+    // Only update if the nextWidth is greater than the prevWidth.
+    if (prevWidth && nextWidth && nextWidth > prevWidth) {
+      return true;
     }
-    const ar = contentRect.bounds.width / contentRect.bounds.height;
-
-    const neitherWidthNorHeightPassed =
-      forcedWidth == null && forcedHeight == null;
-    if (neitherWidthNorHeightPassed) {
-      const width = findNearestWidth(contentRect.bounds.width);
-      const height = Math.ceil(width / ar);
-      return { width, height };
+
+    // Similarly, only update if the next height is greater than
+    // the previous height.
+    if (prevHeight && nextHeight && nextHeight > prevHeight) {
+      return true;
     }
-    if (forcedWidth != null) {
-      const height = Math.ceil(forcedWidth / ar);
-      return { width: forcedWidth, height };
-    } else if (forcedHeight != null) {
-      const width = Math.ceil(forcedHeight * ar);
-      return { width, height: forcedHeight };
+
+    // If we made it here, we need to check if the "top-level"
+    // props have changed (e.g. disableLibraryParam).
+    const shallowPropsEqual = shallowEqual(this.props, nextProps);
+
+    // We also need to check the imgixParams.
+    const shallowParamsEqual = shallowEqual(
+      this.props.imgixParams,
+      nextProps.imgixParams
+    );
+
+    return shallowPropsEqual && shallowParamsEqual;
+  }
+
+  render() {
+    const {
+      measureRef,
+      contentRect,
+      imgixParams = {},
+      onLoad,
+      disableLibraryParam,
+      src,
+      children,
+      className = "",
+    } = this.props;
+    const { w: forcedWidth, h: forcedHeight } = imgixParams;
+    const hasDOMDimensions =
+      contentRect.bounds.width != null && contentRect.bounds.height != null;
+    const htmlAttributes = this.props.htmlAttributes || {};
+    const dpr = toFixed(2, imgixParams.dpr || global.devicePixelRatio || 1);
+    const ref = htmlAttributes.ref;
+    const onRef = (el) => {
+      measureRef(el);
+      if (typeof ref === "function") {
+        ref(el);
+      }
+    };
+
+    const { width, height } = (() => {
+      const bothWidthAndHeightPassed =
+        forcedWidth != null && forcedHeight != null;
+
+      if (bothWidthAndHeightPassed) {
+        return { width: forcedWidth, height: forcedHeight };
+      }
+
+      if (!hasDOMDimensions) {
+        return { width: undefined, height: undefined };
+      }
+      const ar = contentRect.bounds.width / contentRect.bounds.height;
+
+      const neitherWidthNorHeightPassed =
+        forcedWidth == null && forcedHeight == null;
+      if (neitherWidthNorHeightPassed) {
+        const width = findNearestWidth(contentRect.bounds.width);
+        const height = Math.ceil(width / ar);
+        return { width, height };
+      }
+      if (forcedWidth != null) {
+        const height = Math.ceil(forcedWidth / ar);
+        return { width: forcedWidth, height };
+      } else if (forcedHeight != null) {
+        const width = Math.ceil(forcedHeight * ar);
+        return { width, height: forcedHeight };
+      }
+    })();
+    const isReady = width != null && height != null;
+
+    const commonProps = {
+      ...htmlAttributes,
+    };
+
+    if (!isReady) {
+      return (
+        <div
+          {...commonProps}
+          className={`react-imgix-bg-loading ${className}`}
+          ref={onRef}
+        >
+          {children}
+        </div>
+      );
     }
-  })();
-  const isReady = width != null && height != null;
 
-  const commonProps = {
-    ...htmlAttributes,
-  };
+    const renderedSrc = (() => {
+      const [rawSrc, params] = extractQueryParams(src);
+      const srcOptions = {
+        ...params,
+        fit: "crop",
+        ...imgixParams,
+        ...(disableLibraryParam ? {} : { ixlib: `react-${PACKAGE_VERSION}` }),
+        width,
+        height,
+        dpr,
+      };
+
+      return constructUrl(rawSrc, srcOptions);
+    })();
+
+    const style = {
+      ...htmlAttributes.style,
+      backgroundImage: `url(${renderedSrc})`,
+      backgroundSize:
+        (htmlAttributes.style || {}).backgroundSize !== undefined
+          ? htmlAttributes.style.backgroundSize
+          : "cover",
+    };
 
-  if (!isReady) {
     return (
-      <div
-        {...commonProps}
-        className={`react-imgix-bg-loading ${className}`}
-        ref={onRef}
-      >
+      <div {...commonProps} className={className} ref={onRef} style={style}>
         {children}
       </div>
     );
   }
-
-  const renderedSrc = (() => {
-    const [rawSrc, params] = extractQueryParams(src);
-    const srcOptions = {
-      ...params,
-      fit: "crop",
-      ...imgixParams,
-      ...(disableLibraryParam ? {} : { ixlib: `react-${PACKAGE_VERSION}` }),
-      width,
-      height,
-      dpr,
-    };
-
-    return constructUrl(rawSrc, srcOptions);
-  })();
-
-  const style = {
-    ...htmlAttributes.style,
-    backgroundImage: `url(${renderedSrc})`,
-    backgroundSize:
-      (htmlAttributes.style || {}).backgroundSize !== undefined
-        ? htmlAttributes.style.backgroundSize
-        : "cover",
-  };
-
-  return (
-    <div {...commonProps} className={className} ref={onRef} style={style}>
-      {children}
-    </div>
-  );
-};
+}
 const Background = withContentRect("bounds")(BackgroundImpl);
 
 export { Background, BackgroundImpl as __BackgroundImpl };
diff --git a/test/integration/react-imgix.test.jsx b/test/integration/react-imgix.test.jsx
@@ -566,7 +566,7 @@ describe("Background Mode", () => {
     expect(bgImageSrcURL.getQueryParamValue("dpr")).toBe("3.44");
   });
 
-  it("window resize", async () => {
+  it("window resize smaller", async () => {
     const sut = await renderBGAndWaitUntilLoaded(
       <div style={{ width: 1000 }} className="fake-window">
         <style>{`.bg-img { width: 50%; height: 10px}`}</style>
@@ -595,7 +595,61 @@ describe("Background Mode", () => {
     const bgImageSrcURL = findURIfromSUT(sut);
 
     const expectedWidth = BROWSER_WIDTH / 2;
-    expect(bgImageSrcURL.getQueryParamValue("w")).toBe("" + expectedWidth);
+    // We're only resizing if a wider image is required.
+    // If the browser width shrinks, then the "w" we're
+    // getting should be greater than or equal to the
+    // expectedWidth (in this case "w" doesn't change at
+    // all).
+    expect(
+      parseInt(bgImageSrcURL.getQueryParamValue("w"))
+    ).toBeGreaterThanOrEqual(expectedWidth);
+  });
+
+  it("window resize larger", async () => {
+    const sut = await renderBGAndWaitUntilLoaded(
+      <div style={{ width: 1000 }} className="fake-window">
+        <style>{`.bg-img { width: 50%; height: 10px}`}</style>
+        <Background
+          src={`${src}`}
+          imgixParams={{
+            h: 10,
+          }}
+          className="bg-img"
+        >
+          <div>Content</div>
+        </Background>
+      </div>
+    );
+
+    // Before resize:
+    const startWidth = 500;
+    const bgImageSrcURLBefore = findURIfromSUT(sut);
+    expect(parseInt(bgImageSrcURLBefore.getQueryParamValue("w"))).toEqual(
+      startWidth
+    );
+
+    const fakeWindowEl = document.querySelector(".fake-window");
+
+    // Simulate browser resize
+    const BROWSER_WIDTH = 1080;
+    fakeWindowEl.style.width = BROWSER_WIDTH + "px";
+
+    await new Promise((resolve) => {
+      setTimeout(resolve, 1000);
+    });
+
+    const bgImageSrcURL = findURIfromSUT(sut);
+
+    // After resize:
+    const expectedWidth = BROWSER_WIDTH / 2;
+
+    // We've resized, the "w" should be strictly greater than the startWidth.
+    expect(parseInt(bgImageSrcURL.getQueryParamValue("w"))).toBeGreaterThan(
+      startWidth
+    );
+    expect(parseInt(bgImageSrcURL.getQueryParamValue("w"))).toEqual(
+      expectedWidth
+    );
   });
 
   it("can pass ref to component", async () => {