From aab9358d5176f9fb410002105feeb7f6bae9d20c Mon Sep 17 00:00:00 2001 From: Frederick Fogerty Date: Wed, 4 Jul 2018 09:47:04 +1200 Subject: [PATCH] fix: warn the user when no passed as a child to fixes #90 (#151) * test: add failing test to ensure that not passing a fallback image to a element warns the user * fix: warn the user when no passed as a child to * docs(picture): remove documentation about fallback image support --- README.md | 19 +---------- src/react-imgix.js | 61 +++-------------------------------- test/unit/react-imgix.test.js | 55 +++++++++++-------------------- 3 files changed, 24 insertions(+), 111 deletions(-) diff --git a/README.md b/README.md index 1b7d2c03..4d05cac5 100644 --- a/README.md +++ b/README.md @@ -158,24 +158,7 @@ Using the [ element](https://docs.imgix.com/tutorials/using-imgix-pictu ``` -The final `type='img'` component will be created with the options passed into the parent `` container if it's not provided so the above is equivalent to: - -```js - - - - -``` +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 index efe50317..e28d251f 100644 --- 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 and elements are allowable as children of - // 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 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(); + console.warn( + "No fallback image found in the children of a 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 index a0868f34..a59a794a 100644 --- 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( - , - { 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( + , + { 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 passed as a child", () => {