fix: warn the user when no <img> passed as a child to <picture> fixes #…

…90 (#151) * test: add failing test to ensure that not passing a fallback image to a <picture> element warns the user * fix: warn the user when no <img> passed as a child to <picture> * docs(picture): remove documentation about fallback image support
imgix · Jul 3, 2018 · aab9358 · aab9358
1 parent d3183a6
commit aab9358
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 111 deletions.
diff --git a/README.md b/README.md
@@ -158,24 +158,7 @@ Using the [<picture> element](https://docs.imgix.com/tutorials/using-imgix-pictu
 </Imgix>
 ```
 
-The final `type='img'` component will be created with the options passed into the parent `<picture>` container if it's not provided so the above is equivalent to:
-
-```js
-<Imgix src={src} width={100} type="picture">
-  <Imgix
-    src={src}
-    width={400}
-    type="source"
-    imgProps={{ media: "(min-width: 768px)" }}
-  />
-  <Imgix
-    src={src}
-    width={200}
-    type="source"
-    imgProps={{ media: "(min-width: 320px)" }}
-  />
-</Imgix>
-```
+It is highly recommended that a fallback `img` be passed as the last child to a picture component so that the image will render for all screen resolutions.
 
 ## Browser Support
 

diff --git a/src/react-imgix.js b/src/react-imgix.js
@@ -215,7 +215,7 @@ export default class ReactImgix extends Component {
         // we need to make sure an img is the last child so we look for one
         //    in children
         //    a. if we find one, move it to the last entry if it's not already there
-        //    b. if we don't find one, create one.
+        //    b. if we don't find one, warn the user as they probably want to pass one.
 
         // make sure all of our children have key set, otherwise we get react warnings
         _children =
@@ -232,62 +232,9 @@ export default class ReactImgix extends Component {
         );
 
         if (imgIdx === -1) {
-          // didn't find one or empty array - either way make a new component to
-          // put at the end. we pass in almost all of our props as defaults to
-          // our children, exceptions are:
-          //
-          //    bg - only <source> and <img> elements are allowable as children of
-          //         <picture> so we strip this option
-          //    children - we don't want to get recursive here
-          //    component - same reason as bg
-          //    type - specifically we're adding an img type so we hard-code this,
-          //           also letting type=picture through would infinitely loop
-
-          let imgProps = {
-            aggressiveLoad,
-            auto,
-            customParams,
-            crop,
-            entropy,
-            faces,
-            fit,
-            generateSrcSet,
-            src,
-            type: "img",
-            ...other,
-            // make sure to set a unique key too
-            key: buildKey(_children.length + 1)
-          };
-
-          // we also remove className and styles if they exist - those passed in
-          // to our top-level component are set there, if you want them set on
-          // the child <img> element you can use `imgProps`.
-          delete imgProps.className;
-          delete imgProps.styles;
-
-          // ..except if you have passed in imgProps you need those to not disappear,
-          // so we'll remove the imgProps attribute from our imgProps object (ugh!)
-          // and apply them now:
-          imgProps.imgProps = { ...this.props.imgProps };
-          ["className", "styles"].forEach(k => {
-            if (imgProps.imgProps[k]) {
-              imgProps[k] = imgProps.imgProps[k];
-              delete imgProps.imgProps[k];
-            }
-          });
-
-          // have to strip out props set to undefined or empty objects since they
-          // will override any defaultProps in the child
-          Object.keys(imgProps).forEach(k => {
-            if (
-              imgProps[k] === undefined ||
-              (Object.keys(imgProps[k]).length === 0 &&
-                imgProps[k].constructor === Object)
-            )
-              delete imgProps[k];
-          });
-
-          _children.push(<ReactImgix {...imgProps} />);
+          console.warn(
+            "No fallback image found in the children of a <picture> component. A fallback image should be passed to ensure the image renders correctly at all dimensions."
+          );
         } else if (imgIdx !== _children.length - 1) {
           // found one, need to move it to the end
           _children.splice(

diff --git a/test/unit/react-imgix.test.js b/test/unit/react-imgix.test.js
@@ -1,6 +1,6 @@
 import sinon from "sinon";
 import React from "react";
-import ReactDOM from "react-dom";
+import ReactDOM, { render } from "react-dom";
 import { shallow, mount } from "enzyme";
 import PropTypes from "prop-types";
 
@@ -183,43 +183,26 @@ describe("When in picture mode", () => {
     });
   };
 
-  describe("with no children passed as props", () => {
-    const imgProps = { className: "foobar", alt: parentAlt };
-    beforeEach(() => {
-      sut = shallow(
-        <Imgix
-          src={src}
-          type="picture"
-          aggressiveLoad
-          imgProps={imgProps}
-          width={100}
-          height={100}
-        />,
-        { disableLifecycleMethods: true }
-      );
-      children = sut.children();
-      lastChild = children.last();
-    });
+  it("should throw an error when no children passed", () => {
+    const oldConsole = global.console;
+    global.console = { warn: jest.fn() };
 
-    shouldBehaveLikePicture();
+    shallow(
+      <Imgix
+        src={src}
+        type="picture"
+        aggressiveLoad
+        width={100}
+        height={100}
+      />,
+      { disableLifecycleMethods: true }
+    );
 
-    it("only one child should exist", () => {
-      expect(children.length).toBe(1);
-    });
-
-    it("props should be passed down to the automatically added element, and type should be img", () => {
-      // todo - verify all valid props are passed down to children as defaults
-      // except for the ones we specifically exclude
-      let expectedProps = {
-        ...sut.props(),
-        type: "img",
-        imgProps
-      };
-      expectedProps.className = expectedProps.imgProps.className;
-      delete expectedProps.children;
-      delete expectedProps.imgProps.className;
-      expect(lastChild.props()).toMatchObject(expectedProps);
-    });
+    expect(console.warn).toHaveBeenCalledWith(
+      expect.stringContaining("No fallback image found")
+    );
+
+    global.console = oldConsole;
   });
 
   describe("with a <ReactImgix type=img> passed as a child", () => {