Repository: gatsbyjs/gatsby
File: packages/gatsby-image/src/index.js (JavaScript)

Code before:
 * @param currentData  {{media?: string}[]}   The props to check for images.
 * @return {boolean}
 */
const hasArtDirectionSupport = currentData =>
  !!currentData &&
  Array.isArray(currentData) &&
  currentData.some(image => typeof image.media !== `undefined`)

/**
 * Tries to detect if a media query matches the current viewport.
 * @property media   {{media?: string}}  A media query string.
 * @return {boolean}
 */
const matchesMedia = ({ media }) =>
  media ? isBrowser && !!window.matchMedia(media).matches : false

/**
 * 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}
 */
const getImageSrcKey = ({ fluid, fixed }) => {
  const currentData = fluid || fixed

  return currentData && getCurrentSrcData(currentData).src
}

/**
 * Returns the current src - Preferably with art-direction support.
 * @param currentData  {{media?: string}[], maxWidth?: Number, maxHeight?: Number}   The fluid or fixed image array.
 * @return {{src: string, media?: string, maxWidth?: Number, maxHeight?: Number}}
 */
const getCurrentSrcData = currentData => {
  if (isBrowser && hasArtDirectionSupport(currentData)) {
    // Do we have an image for the current Viewport?
    const foundMedia = currentData.findIndex(matchesMedia)
    if (foundMedia !== -1) {
      return currentData[foundMedia]
    }

    // No media matches, select first element without a media condition
    const noMedia = currentData.findIndex(
      image => typeof image.media === `undefined`
    )
    if (noMedia !== -1) {
      return currentData[noMedia]
    }
  }
  // Else return the first image.
  return currentData[0]

Review comment (bug):
Is this where the error is actually occurring, or within the function call logic itself?

I see `getCurrentSrcData()` use a conditional, with the 2nd condition being a method that runs checks, those would abort and not trigger further errors, except for the end of the function, where it's assumed an array has been provided to the function. The other method callers are in the fluid/fixed condition checks in render method, they should be fine.

The error here is from trying to access the `src` property? Your fix will return false otherwise, where `getImageSrcKey()` would be affected when creating a cache entry under `false` key. This can apparently be a valid key for an object, which may cause a problem if several `<Img />` utilize the same cache key?(I'm not sure if the cache object is shared among instances)

https://github.com/gatsbyjs/gatsby/blob/6102456c4a9f0a09aad5e3c2d61d6a38591ded2d/packages/gatsby-image/src/index.js#L120

If that would be an issue we'd want to address that.

---

I think this is more clear for where guard is appropriate:

```js
  const currentData = getCurrentSrcData(fluid || fixed)
  return currentData && currentData.src
```

Probably still want to avoid caching `false` as a cache key though if that's returned instead.

---

https://github.com/gatsbyjs/gatsby/blob/6102456c4a9f0a09aad5e3c2d61d6a38591ded2d/packages/gatsby-image/src/index.js#L103-L104

`hasArtDirectionSupport()` is only called by one method atm, the method that ends with the above under the assumption there is an array of objects provided.

https://github.com/gatsbyjs/gatsby/blob/6102456c4a9f0a09aad5e3c2d61d6a38591ded2d/packages/gatsby-image/src/index.js#L58-L59

You could add this to the start of `getCurrentSrcData()` to abort early with false bool.

---

Alternatively, is this something that the component should actually support? (No fluid or fixed object provided) Or should an error be thrown here(or earlier) instead? (albeit with a friendlier message)

Code after:
 * @param currentData  {{media?: string}[]}   The props to check for images.
 * @return {boolean}
 */
const hasArtDirectionSupport = currentData =>
  !!currentData &&
  Array.isArray(currentData) &&
  currentData.some(image => typeof image.media !== `undefined`)

/**
 * Tries to detect if a media query matches the current viewport.
 * @property media   {{media?: string}}  A media query string.
 * @return {boolean}
 */
const matchesMedia = ({ media }) =>
  media ? isBrowser && !!window.matchMedia(media).matches : false

/**
 * 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?} Returns image src or undefined it not given.
 */
const getImageCacheKey = ({ fluid, fixed }) => {
  const srcData = getCurrentSrcData(fluid || fixed || [])

  return srcData && srcData.src
}

/**
 * Returns the current src - Preferably with art-direction support.
 * @param currentData  {{media?: string}[], maxWidth?: Number, maxHeight?: Number}   The fluid or fixed image array.
 * @return {{src: string, media?: string, maxWidth?: Number, maxHeight?: Number}}
 */
const getCurrentSrcData = currentData => {
  if (isBrowser && hasArtDirectionSupport(currentData)) {
    // Do we have an image for the current Viewport?
    const foundMedia = currentData.findIndex(matchesMedia)
    if (foundMedia !== -1) {
      return currentData[foundMedia]
    }

    // No media matches, select first element without a media condition
    const noMedia = currentData.findIndex(
      image => typeof image.media === `undefined`
    )
    if (noMedia !== -1) {
      return currentData[noMedia]
    }
  }
  // Else return the first image.
  return currentData[0]