feat: integrate @imgix/js-core into react-imgix (#780)

* build: add @imgix/js-core as a dependency * feat: impl targetWidths in terms of ImgixClient.targetWidths() * feat: impl buildSrcSet in terms of @imgix/js-core * fix: test that issue #763 has been fixed * refactor: remove dead code * refactor: tidy up * fix: no need to include w or h if building fluid-width srcset
imgix · Apr 22, 2021 · 690e7b6 · 690e7b6
1 parent 0d52511
commit 690e7b6
Show file tree

Hide file tree

Showing 9 changed files with 167 additions and 2,103 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -55,7 +55,7 @@
     "react-dom": "15.x || 16.x || 17.x"
   },
   "dependencies": {
-    "js-base64": "~2.6.0",
+    "@imgix/js-core": "^3.1.3",
     "react-measure": "^2.3.0",
     "shallowequal": "^1.1.0",
     "warning": "^4.0.1"

diff --git a/src/constants.js b/src/constants.js
@@ -1,11 +1 @@
-export const DPR_QUALITY = {
-  q_dpr1: 75,
-  q_dpr2: 50,
-  q_dpr3: 35,
-  q_dpr4: 23,
-  q_dpr5: 20,
-};
-
 export const PACKAGE_VERSION = "9.0.4";
-
-export const DPR_QUALITY_VALUES = Object.values(DPR_QUALITY);
diff --git a/src/constructUrl.js b/src/constructUrl.js
@@ -5,9 +5,9 @@ Licensed under the Apache License 2.0, seen https://github.com/coursera/react-im
 Minor syntax modifications have been made
 */
 
-var Base64 = require("js-base64").Base64;
 const PACKAGE_VERSION = require("../package.json").version;
 import extractQueryParams from "./extractQueryParams";
+import ImgixClient from "@imgix/js-core";
 
 // @see https://www.imgix.com/docs/reference
 var PARAM_EXPANSION = Object.freeze({
@@ -116,28 +116,39 @@ function constructUrl(src, longOptions) {
   if (!src) {
     return "";
   }
+  const params = compactParamKeys(longOptions);
+  const { client, pathComponents } = extractClientAndPathComponents(src);
+  return client.buildURL(pathComponents.join("/"), params);
+}
 
+function compactParamKeys(longOptions) {
   const keys = Object.keys(longOptions);
   const keysLength = keys.length;
-  let url = src + "?";
+  const params = {};
   for (let i = 0; i < keysLength; i++) {
     let key = keys[i];
-    let val = longOptions[key];
-
     if (PARAM_EXPANSION[key]) {
-      key = PARAM_EXPANSION[key];
+      params[PARAM_EXPANSION[key]] = longOptions[key];
     } else {
-      key = encodeURIComponent(key);
+      params[key] = longOptions[key];
     }
+  }
 
-    if (key.substr(-2) === "64") {
-      val = Base64.encodeURI(val);
-    }
+  return params;
+}
 
-    url += key + "=" + encodeURIComponent(val) + "&";
-  }
+function extractClientAndPathComponents(src) {
+  const [scheme, rest] = src.split("://");
+  const [domain, ...pathComponents] = rest.split("/");
+  let useHTTPS = scheme == "https";
+
+  const client = new ImgixClient({
+    domain: domain,
+    useHTTPS: useHTTPS,
+    includeLibraryParam: false,
+  });
 
-  return url.slice(0, -1);
+  return { client, pathComponents };
 }
 
 function buildURLPublic(src, imgixParams = {}, options = {}) {
@@ -158,4 +169,4 @@ function buildURLPublic(src, imgixParams = {}, options = {}) {
 
 export default constructUrl;
 
-export { buildURLPublic };
+export { buildURLPublic, compactParamKeys, extractClientAndPathComponents };
diff --git a/src/react-imgix-bg.jsx b/src/react-imgix-bg.jsx
@@ -1,12 +1,11 @@
 import React from "react";
 import { withContentRect } from "react-measure";
-import { PACKAGE_VERSION } from './constants';
+import { PACKAGE_VERSION } from "./constants";
 import constructUrl from "./constructUrl";
 import extractQueryParams from "./extractQueryParams";
 import findClosest from "./findClosest";
 import targetWidths from "./targetWidths";
 
-
 const noop = () => {};
 
 const findNearestWidth = (actualWidth) =>

diff --git a/src/react-imgix.jsx b/src/react-imgix.jsx
@@ -2,11 +2,13 @@ import PropTypes from "prop-types";
 import React, { Component } from "react";
 import "./array-findindex";
 import { compose, config } from "./common";
-import { DPR_QUALITY_VALUES, PACKAGE_VERSION } from "./constants";
-import constructUrl from "./constructUrl";
+import { PACKAGE_VERSION } from "./constants";
+import constructUrl, {
+  compactParamKeys,
+  extractClientAndPathComponents,
+} from "./constructUrl";
 import extractQueryParams from "./extractQueryParams";
 import { ShouldComponentUpdateHOC } from "./HOCs";
-import targetWidths from "./targetWidths";
 
 const NODE_ENV = process.env.NODE_ENV;
 
@@ -69,20 +71,11 @@ const setParentRef = (parentRef, el) => {
   }
 };
 
-const buildSrcSetPairWithFixedHeight = (url, targetWidth, fixedHeight, _) =>
-  url + "&h=" + fixedHeight + "&w=" + targetWidth + " " + targetWidth + "w";
-
-const buildSrcSetPairWithTargetWidth = (url, targetWidth, _1, _2) =>
-  url + "&w=" + targetWidth + " " + targetWidth + "w";
-
-const buildDprSrcWithQuality = (url, quality, dpr) =>
-  url + "&q=" + quality + "&dpr=" + dpr + " " + dpr + "x";
-
-const buildDprSrcWithoutQuality = (url, _, dpr) =>
-  url + "&dpr=" + dpr + " " + dpr + "x";
-
-const buildDprSrcWithQualityByDpr = (url, quality, dpr) =>
-  url + "&q=" + quality + "&dpr=" + dpr + " " + dpr + "x";
+function buildSrcSet(rawSrc, params = {}, options = {}) {
+  const { client, pathComponents } = extractClientAndPathComponents(rawSrc);
+  const compactedParams = compactParamKeys(params);
+  return client.buildSrcSet(pathComponents.join("/"), compactedParams, options);
+}
 
 /**
  * Build a imgix source url with parameters from a raw url
@@ -117,43 +110,33 @@ function buildSrc({
     srcSet = src;
   } else {
     if (fixedSize) {
-      const { q, ...urlParams } = srcOptions;
-      const constructedUrl = constructUrl(rawSrc, urlParams);
-
-      let srcFn = buildDprSrcWithQualityByDpr;
+      const { width, w, height, h, q, ...urlParams } = srcOptions;
       if (q) {
-        srcFn = buildDprSrcWithQuality;
-      } else if (disableQualityByDPR) {
-        srcFn = buildDprSrcWithoutQuality;
+        urlParams["q"] = q;
       }
 
-      srcSet = "";
-      const len = DPR_QUALITY_VALUES.length;
-      for (let i = 0; i < len; i++) {
-        const quality = DPR_QUALITY_VALUES[i];
-        srcSet += srcFn(constructedUrl, q || quality, i + 1) + ", ";
+      const finalWidth = width || w;
+      const finalHeight = height || h;
+
+      if (finalWidth) {
+        urlParams["w"] = finalWidth;
       }
-      srcSet = srcSet.slice(0, -2);
+
+      if (finalHeight) {
+        urlParams["h"] = finalHeight;
+      }
+
+      srcSet = buildSrcSet(rawSrc, urlParams, {
+        disableVariableQuality: disableQualityByDPR,
+      });
     } else {
-      const { width, w, height, ...urlParams } = srcOptions;
-      const constructedUrl = constructUrl(rawSrc, urlParams);
+      const { width, w, height, h, ...urlParams } = srcOptions;
 
       const aspectRatio = imgixParams.ar;
       let showARWarning =
         aspectRatio != null && aspectRatioIsValid(aspectRatio) === false;
 
-      let srcFn = buildSrcSetPairWithTargetWidth;
-      if (height) {
-        srcFn = buildSrcSetPairWithFixedHeight;
-      }
-
-      srcSet = "";
-      const len = targetWidths.length;
-      for (let i = 0; i < len; i++) {
-        const targetWidth = targetWidths[i];
-        srcSet += srcFn(constructedUrl, targetWidth, height) + ", ";
-      }
-      srcSet = srcSet.slice(0, -2);
+      srcSet = buildSrcSet(rawSrc, urlParams);
 
       if (
         NODE_ENV !== "production" &&

diff --git a/src/targetWidths.js b/src/targetWidths.js
@@ -1,18 +1,3 @@
-function targetWidths() {
-  const resolutions = [];
-  let prev = 100;
-  const INCREMENT_PERCENTAGE = 8;
-  const MAX_SIZE = 8192;
+import ImgixClient from "@imgix/js-core";
 
-  const ensureEven = (n) => 2 * Math.round(n / 2);
-
-  while (prev <= MAX_SIZE) {
-    resolutions.push(ensureEven(prev));
-    prev *= 1 + (INCREMENT_PERCENTAGE / 100) * 2;
-  }
-
-  resolutions.push(MAX_SIZE);
-  return resolutions;
-}
-
-export default targetWidths();
+export default ImgixClient.targetWidths();
diff --git a/test/integration/react-imgix.test.jsx b/test/integration/react-imgix.test.jsx
@@ -397,6 +397,33 @@ describe("Background Mode", () => {
     });
   });
 
+  describe("when both w and h provided", () => {
+    it("does not duplicate query params for height and width", () => {
+      const sut = renderIntoContainer(
+        <Background
+          src="https://assets.imgix.net/examples/pione.jpg"
+          imgixParams={{
+            w: 300,
+            h: 350,
+          }}
+          className="bg-img"
+        ></Background>
+      );
+
+      const bgImageSrcURL = findURIfromSUT(sut);
+      const seen = new Set();
+      for (const [k, _v] of bgImageSrcURL.queryPairs) {
+        if (seen.has(k)) {
+          throw new Error(
+            `duplicate keys for '${k}' found in query parameters`
+          );
+        } else {
+          seen.add(k);
+        }
+      }
+    });
+  });
+
   describe("without the backgroundSize prop set", () => {
     beforeEach(async () => {
       sut = await renderBGAndWaitUntilLoaded(

diff --git a/test/unit/react-imgix.test.jsx b/test/unit/react-imgix.test.jsx
@@ -3,7 +3,6 @@ import React from "react";
 import { shallow as enzymeShallow, mount } from "enzyme";
 import { shallowUntilTarget } from "../helpers";
 import targetWidths from "targetWidths";
-import { DPR_QUALITY } from "../../src/constants";
 
 import Imgix, {
   __ReactImgixImpl,
@@ -14,6 +13,14 @@ import Imgix, {
 } from "react-imgix";
 import { __BackgroundImpl } from "react-imgix-bg";
 
+const DPR_QUALITY = {
+  q_dpr1: 75,
+  q_dpr2: 50,
+  q_dpr3: 35,
+  q_dpr4: 23,
+  q_dpr5: 20,
+};
+
 function shallow(element, target = __ReactImgixImpl, shallowOptions) {
   return shallowUntilTarget(element, target, {
     shallowOptions: shallowOptions || {
@@ -62,10 +69,10 @@ describe("When in default mode", () => {
       const sut = shallow(<Imgix src={src} sizes="100vw" />);
       const srcset = sut.props().srcSet;
       expect(srcset).not.toBeUndefined();
-      expect(srcset.split(", ")[0].split(" ")).toHaveLength(2);
-      const aSrcFromSrcSet = srcset.split(", ")[0].split(" ")[0];
+      expect(srcset.split(",\n")[0].split(" ")).toHaveLength(2);
+      const aSrcFromSrcSet = srcset.split(",\n")[0].split(" ")[0];
       expect(aSrcFromSrcSet).toContain(src);
-      const aWidthFromSrcSet = srcset.split(", ")[0].split(" ")[1];
+      const aWidthFromSrcSet = srcset.split(",\n")[0].split(" ")[1];
       expect(aWidthFromSrcSet).toMatch(/^\d+w$/);
     });
     it("returns the expected number of `url widthDescriptor` pairs", function () {
@@ -117,7 +124,7 @@ describe("When in default mode", () => {
     describe("supports varying q to dpr matching when rendering a fixed-size image", () => {
       it("generates predefined q and dpr pairs", async () => {
         const sut = shallow(<Imgix src={src} width={100} />);
-        const srcset = sut.props().srcSet.split(", ");
+        const srcset = sut.props().srcSet.split(",\n");
 
         expect(srcset[0].split(" ")[0]).toContain("q=" + DPR_QUALITY.q_dpr1);
         expect(srcset[1].split(" ")[0]).toContain("q=" + DPR_QUALITY.q_dpr2);
@@ -129,7 +136,7 @@ describe("When in default mode", () => {
         const sut = shallow(
           <Imgix src={src} width={100} disableQualityByDPR />
         );
-        const srcset = sut.props().srcSet.split(", ");
+        const srcset = sut.props().srcSet.split(",\n");
 
         expect(srcset[0].split(" ")[0]).not.toContain(
           "q=" + DPR_QUALITY.q_dpr1
@@ -152,7 +159,7 @@ describe("When in default mode", () => {
         const sut = shallow(
           <Imgix src={src} width={100} imgixParams={{ q: q_override }} />
         );
-        const srcset = sut.props().srcSet.split(", ");
+        const srcset = sut.props().srcSet.split(",\n");
 
         expect(srcset[0].split(" ")[0]).toContain("q=" + q_override);
         expect(srcset[1].split(" ")[0]).toContain("q=" + q_override);
@@ -230,10 +237,10 @@ describe("When in <source> mode", () => {
     it("should have a srcSet set correctly", async () => {
       const srcset = renderImage().props().srcSet;
       expect(srcset).not.toBeUndefined();
-      expect(srcset.split(", ")[0].split(" ")).toHaveLength(2);
-      const aSrcFromSrcSet = srcset.split(", ")[0].split(" ")[0];
+      expect(srcset.split(",\n")[0].split(" ")).toHaveLength(2);
+      const aSrcFromSrcSet = srcset.split(",\n")[0].split(" ")[0];
       expect(aSrcFromSrcSet).toContain(src);
-      const aWidthFromSrcSet = srcset.split(", ")[0].split(" ")[1];
+      const aWidthFromSrcSet = srcset.split(",\n")[0].split(" ")[1];
       expect(aWidthFromSrcSet).toMatch(/^\d+w$/);
     });
 
@@ -275,7 +282,7 @@ describe("When in <source> mode", () => {
     it("srcSet should be in the form src 1x, src 2x, src 3x, src 4x, src 5x", () => {
       const srcSet = renderImage().props().srcSet;
 
-      const srcSets = srcSet.split(", ");
+      const srcSets = srcSet.split(",\n");
       expect(srcSets).toHaveLength(5);
       srcSets.forEach((srcSet) => {
         expect(srcSet).toContain(src);