Skip to content

Commit

Permalink
fix(gatsby-image): add required check for one of fluid or fixed (#25371)
Browse files Browse the repository at this point in the history
* Do not crash the page if the image component is invoked without parameters

* added a warning if missing fluid and fixed props

* had swapped a condition

* doing destructuring in function parameter list

* now with functioning tree-shaking

* updating function doc: returns undefined, not null

* switching to proptypes for validation

* Update packages/gatsby-image/src/index.js

Co-authored-by: Andreas Ehrencrona <andreas@scholarsapp.com>
Co-authored-by: Ward Peeters <ward@coding-tech.com>
  • Loading branch information
3 people committed Jul 29, 2020
1 parent a94e657 commit ce7ba8a
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 12 deletions.
12 changes: 12 additions & 0 deletions packages/gatsby-image/src/__tests__/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,18 @@ describe(`<Image />`, () => {
expect(console.warn).toBeCalled()
})

it(`should warn if missing both fixed and fluid props`, () => {
jest.spyOn(global.console, `error`)

render(<Image fixed={null} />)

expect(console.error).toBeCalledWith(
expect.stringContaining(
`The prop \`fluid\` or \`fixed\` is marked as required`
)
)
})

it(`should select the correct mocked image of fluid variants provided.`, () => {
const tripleFluidImageShapeMock = fluidImagesShapeMock.concat({
aspectRatio: 5,
Expand Down
49 changes: 37 additions & 12 deletions packages/gatsby-image/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,12 @@ const matchesMedia = ({ media }) =>
* Find the source of an image to use as a key in the image cache.
* Use `the first image in either `fixed` or `fluid`
* @param {{fluid: {src: string, media?: string}[], fixed: {src: string, media?: string}[]}} args
* @return {string}
* @return {string?} Returns image src or undefined it not given.
*/
const getImageSrcKey = ({ fluid, fixed }) => {
const data = fluid ? getCurrentSrcData(fluid) : getCurrentSrcData(fixed)
const getImageCacheKey = ({ fluid, fixed }) => {
const srcData = getCurrentSrcData(fluid || fixed || [])

return data.src
return srcData && srcData.src
}

/**
Expand Down Expand Up @@ -109,16 +109,20 @@ const getCurrentSrcData = currentData => {
const imageCache = Object.create({})
const inImageCache = props => {
const convertedProps = convertProps(props)
// Find src
const src = getImageSrcKey(convertedProps)
return imageCache[src] || false

const cacheKey = getImageCacheKey(convertedProps)

return imageCache[cacheKey] || false
}

const activateCacheForImage = props => {
const convertedProps = convertProps(props)
// Find src
const src = getImageSrcKey(convertedProps)
imageCache[src] = true

const cacheKey = getImageCacheKey(convertedProps)

if (cacheKey) {
imageCache[cacheKey] = true
}
}

// Native lazy-loading support: https://addyosmani.com/blog/lazy-loading/
Expand Down Expand Up @@ -723,15 +727,36 @@ const fluidObject = PropTypes.shape({
maxHeight: PropTypes.number,
})

function requireFixedOrFluid(originalPropTypes) {
return (props, propName, componentName) => {
if (!props.fixed && !props.fluid) {
throw new Error(
`The prop \`fluid\` or \`fixed\` is marked as required in \`${componentName}\`, but their values are both \`undefined\`.`
)
}

PropTypes.checkPropTypes(
{ [propName]: originalPropTypes },
props,
`prop`,
componentName
)
}
}

// If you modify these propTypes, please don't forget to update following files as well:
// https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby-image/index.d.ts
// https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby-image/README.md#gatsby-image-props
// https://github.com/gatsbyjs/gatsby/blob/master/docs/docs/gatsby-image.md#gatsby-image-props
Image.propTypes = {
resolutions: fixedObject,
sizes: fluidObject,
fixed: PropTypes.oneOfType([fixedObject, PropTypes.arrayOf(fixedObject)]),
fluid: PropTypes.oneOfType([fluidObject, PropTypes.arrayOf(fluidObject)]),
fixed: requireFixedOrFluid(
PropTypes.oneOfType([fixedObject, PropTypes.arrayOf(fixedObject)])
),
fluid: requireFixedOrFluid(
PropTypes.oneOfType([fluidObject, PropTypes.arrayOf(fluidObject)])
),
fadeIn: PropTypes.bool,
durationFadeIn: PropTypes.number,
title: PropTypes.string,
Expand Down

0 comments on commit ce7ba8a

Please sign in to comment.