Skip to content

Commit

Permalink
fix(789): ensure children passed to background component update
Browse files Browse the repository at this point in the history
This commit refactors the `shouldComponentUpdate` function outside of
the `BackgroundImpl` component.

The refactored function, `__shouldComponentUpdate` takes `props` and
`nextProps` as arguments and returns true if the children, imgixParams,
contextRect, bounds, htmlAttributes, or top-level props have changed.
It returns false if the props have not changed.

The refactoring addresses an issue where changes to the children and
imgixParams props were not detected and the child components therefore
did not re-render.

Co-authored-by: Frederick Fogerty <frederick@imgix.com>
  • Loading branch information
luqven and Frederick Fogerty committed May 28, 2021
1 parent 212574a commit 643b809
Showing 1 changed file with 55 additions and 33 deletions.
88 changes: 55 additions & 33 deletions src/react-imgix-bg.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,57 +7,79 @@ import findClosest from "./findClosest";
import targetWidths from "./targetWidths";
import { shallowEqual } from "./common";

const noop = () => {};

const findNearestWidth = (actualWidth) =>
findClosest(actualWidth, targetWidths);

const toFixed = (dp, value) => +value.toFixed(dp);

class BackgroundImpl extends React.Component {
constructor(props) {
super(props);
export const __shouldComponentUpdate = (props, nextProps) => {
const contentRect = props.contentRect;
const bounds = contentRect.bounds;
const { width: prevWidth, height: prevHeight } = bounds;

const nextContentRect = nextProps.contentRect;
const nextBounds = nextContentRect.bounds;
const { width: nextWidth, height: nextHeight } = nextBounds;

// If neither of the previous nor next dimensions are present,
// re-render.
if (!nextWidth || !nextHeight || !prevWidth || !prevHeight) {
return true;
}

shouldComponentUpdate(nextProps) {
const contentRect = this.props.contentRect;
const bounds = contentRect.bounds;
const { width: prevWidth, height: prevHeight } = bounds;
// 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 nextContentRect = nextProps.contentRect;
const nextBounds = nextContentRect.bounds;
const { width: nextWidth, height: nextHeight } = nextBounds;
// Similarly, only update if the next height is greater than
// the previous height.
if (prevHeight && nextHeight && nextHeight > prevHeight) {
return true;
}

// If neither of the previous nor next dimensions are present,
// re-render.
if (!nextWidth || !nextHeight || !prevWidth || !prevHeight) {
const customizer = (oldProp, newProp, key) => {
// these keys are ignored from prop checking process
if (key === "contextRect" || key === "measure" || key === "measureRef") {
return true;
}

// 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;
if (key === "children") {
return oldProp == newProp;
}

// Similarly, only update if the next height is greater than
// the previous height.
if (prevHeight && nextHeight && nextHeight > prevHeight) {
return true;
if (key === "imgixParams") {
return shallowEqual(oldProp, newProp, (a, b) => {
if (Array.isArray(a)) {
return shallowEqual(a, b);
}
return undefined;
});
}

// 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);
if (key === "htmlAttributes") {
return shallowEqual(oldProp, newProp);
}

// We also need to check the imgixParams.
const shallowParamsEqual = shallowEqual(
this.props.imgixParams,
nextProps.imgixParams
);
return undefined; // handled by shallowEqual
};

// If we made it here, we need to check if the "top-level"
// props have changed (e.g. disableLibraryParam).
const propsEqual = shallowEqual(props, nextProps, customizer);

return shallowPropsEqual && shallowParamsEqual;
return !(propsEqual);
}

class BackgroundImpl extends React.Component {
constructor(props) {
super(props);
}

shouldComponentUpdate(nextProps) {
return __shouldComponentUpdate(this.props, nextProps);
}

render() {
Expand Down

0 comments on commit 643b809

Please sign in to comment.