feat: allow invalid source data by making gatsbyImageData nullable (#218)

When sourcing data every sourced item might _not_ be be valid. Instead
of stopping the build an error will be logged.

Make sure to check if `gatsbyImageData` is not null before using in the
`GatsbyImage` component.

Closes #214

Author: raae

demo/gatsby-config.js

@@ -31,6 +31,7 @@ module.exports = {
           'CloudinaryAsset',
           'ExistingDataExampleImage',
           'ExistingDataNestedExampleImage',
+          'SomeBadImageData',
         ],
       },
     },

demo/gatsby-node.js

@@ -39,6 +39,25 @@ exports.sourceNodes = (gatsbyUtils) => {
 
   reporter.info(`[site] Create ExistingData node # 1`);
 
+  createNode({
+    id: createNodeId(`GoodData >>> 1`),
+    name: 'GoodData',
+    ...cloudinaryData1,
+    internal: {
+      type: 'SomeBadImageData',
+      contentDigest: createContentDigest('GoodData' + cloudinaryData1),
+    },
+  });
+
+  createNode({
+    id: createNodeId(`BadData >>> 2`),
+    name: 'BadData',
+    internal: {
+      type: 'SomeBadImageData',
+      contentDigest: createContentDigest('BadData'),
+    },
+  });
+
   const cloudinaryData2 = {
     cloudName: 'jlengstorf',
     publicId: 'gatsby-cloudinary/jason',

demo/src/pages/somebaddata.js

@@ -0,0 +1,28 @@
+import React from 'react';
+import { graphql, useStaticQuery } from 'gatsby';
+import { GatsbyImage, getImage } from 'gatsby-plugin-image';
+
+const ProblemExample = () => {
+  const data = useStaticQuery(graphql`
+    query {
+      allSomeBadImageData {
+        nodes {
+          name
+          gatsbyImageData(height: 200)
+        }
+      }
+    }
+  `);
+
+  return data.allSomeBadImageData.nodes.map((node, index) => {
+    const gatsbyImage = getImage(node);
+
+    if (gatsbyImage) {
+      return <GatsbyImage key={index} image={gatsbyImage} alt={node.name} />;
+    } else {
+      return <div>No image for node with name: {node.name}</div>;
+    }
+  });
+};
+
+export default ProblemExample;

plugin/gatsby-plugin-image/index.js

@@ -10,6 +10,8 @@ exports.createGatsbyImageDataResolver = (gatsbyUtils, pluginOptions) => {
 
   if (gatsbyImageResolver) {
     const resolvers = {};
+    // Make the resolver nullable, createGatsbyPluginImageResolver sets the type to 'GatsbyImageData!'
+    gatsbyImageResolver.type = 'GatsbyImageData';
 
     transformTypes.forEach((type) => {
       // Add gatsbyImageData resolver

plugin/gatsby-plugin-image/resolve-asset.js

@@ -10,12 +10,12 @@ const {
   getAssetMetadata,
 } = require('./asset-data');
 
-exports._generateCloudinaryAssetSource = (
+const generateCloudinaryAssetSource = (
   filename,
   width,
   height,
   format,
-  fit,
+  _fit,
   options
 ) => {
   const [cloudName, publicId] = filename.split('>>>');
@@ -38,9 +38,21 @@ exports._generateCloudinaryAssetSource = (
   return imageSource;
 };
 
+exports._generateCloudinaryAssetSource = generateCloudinaryAssetSource;
+
 exports.createResolveCloudinaryAssetData =
-  (gatsbyUtils) => async (source, args) => {
+  (gatsbyUtils) => async (source, args, _context, info) => {
     const { reporter } = gatsbyUtils;
+    const transformType = info.parentType || 'UnknownTransformType';
+
+    const hasRequiredData = source.cloudName && source.publicId;
+
+    if (!hasRequiredData) {
+      reporter.error(
+        `[gatsby-transformer-cloudinary] Missing required fields on ${transformType}: cloudName=${source.cloudName}, publicId=${source.cloudName}`
+      );
+      return null;
+    }
 
     let metadata = {
       width: source.originalWidth,
@@ -54,18 +66,16 @@ exports.createResolveCloudinaryAssetData =
     if (!hasSizingAndFormatMetadata) {
       // Lacking metadata, so lets request it from Cloudinary
       try {
+        metadata = await getAssetMetadata({ source, args });
         reporter.verbose(
-          `[gatsby-transformer-cloudinary] Missing metadata on ${source.cloudName} > ${source.publicId}`
-        );
-        reporter.verbose(
-          `[gatsby-transformer-cloudinary] >>> To save on network requests add originalWidth, originalHeight and originalFormat to the asset data`
+          `[gatsby-transformer-cloudinary] Missing metadata fields on ${transformType} for ${source.cloudName} > ${source.publicId}
+          >>> To save on network requests add originalWidth, originalHeight and originalFormat to ${transformType}`
         );
-        metadata = await getAssetMetadata({ source, args });
       } catch (error) {
-        reporter.panic(
-          `[gatsby-transformer-cloudinary] Could not get metadata for ${source.cloudName} > ${source.publicId}:`,
-          error
+        reporter.error(
+          `[gatsby-transformer-cloudinary] Could not get metadata for ${transformType} for ${source.cloudName} > ${source.publicId}: ${error.message}`
         );
+        return null;
       }
     }
 
@@ -75,7 +85,7 @@ exports.createResolveCloudinaryAssetData =
       // Passing the plugin name allows for better error messages
       pluginName: `gatsby-transformer-cloudinary`,
       sourceMetadata: metadata,
-      generateImageSource: this._generateCloudinaryAssetSource,
+      generateImageSource: generateCloudinaryAssetSource,
       options: args,
     };
 
@@ -98,8 +108,7 @@ exports.createResolveCloudinaryAssetData =
       }
     } catch (error) {
       reporter.error(
-        `[gatsby-transformer-cloudinary] Could not generate placeholder (${args.placeholder}) for ${source.cloudName} > ${source.publicId}:`,
-        error
+        `[gatsby-transformer-cloudinary] Could not generate placeholder (${args.placeholder}) for ${source.cloudName} > ${source.publicId}: ${error.message}`
       );
     }
 

plugin/gatsby-plugin-image/resolve-asset.test.js

@@ -3,8 +3,9 @@ jest.mock('./asset-data');
 
 const gatsbyUtilsMocks = {
   reporter: {
-    error: jest.fn(),
     panic: jest.fn(),
+    error: jest.fn(),
+    info: jest.fn(),
     verbose: jest.fn(),
   },
 };
@@ -58,18 +59,22 @@ describe('generateCloudinaryAssetSource', () => {
 });
 
 describe('resolveCloudinaryAssetData', () => {
-  const source = {
+  const sourceWithMetadata = {
     publicId: 'public-id',
     cloudName: 'cloud-name',
     originalWidth: '600',
     originalHeight: '300',
     originalFormat: 'jpg',
   };
 
-  const args = {
-    transformations: ['e_grayscale'],
+  const sourceWithoutMeta = {
+    publicId: 'public-id',
+    cloudName: 'cloud-name',
   };
 
+  const context = {}; // Never used
+  const info = {};
+
   beforeEach(() => {
     getAssetMetadata.mockResolvedValue({
       width: 100,
@@ -85,13 +90,15 @@ describe('resolveCloudinaryAssetData', () => {
     jest.clearAllMocks();
   });
 
-  it('calls gatsby-plugin-image -> generateImageData', async () => {
-    await resolveCloudinaryAssetData(source, args);
+  it('calls gatsby-plugin-image -> generateImageData once', async () => {
+    const args = {};
+    await resolveCloudinaryAssetData(sourceWithMetadata, args, context, info);
     expect(generateImageData).toBeCalledTimes(1);
   });
 
   it('calls gatsby-plugin-image -> generateImageData with correct data', async () => {
-    await resolveCloudinaryAssetData(source, args);
+    const args = { transformations: ['e_grayscale'] };
+    await resolveCloudinaryAssetData(sourceWithMetadata, args, context, info);
     expect(generateImageData).toBeCalledWith({
       filename: 'cloud-name>>>public-id',
       generateImageSource: _generateCloudinaryAssetSource,
@@ -109,12 +116,13 @@ describe('resolveCloudinaryAssetData', () => {
   });
 
   it('fetches metadata when not present on source', async () => {
-    await resolveCloudinaryAssetData(source, {});
-    await resolveCloudinaryAssetData(
-      { publicId: 'public-id', cloudName: 'cloud-name' },
-      {}
-    );
+    const args = {};
+    await resolveCloudinaryAssetData(sourceWithMetadata, args, context, info);
+    await resolveCloudinaryAssetData(sourceWithoutMeta, args, context, info);
+    // getAssetMetadata should only be called for sourceWithoutMeta
     expect(getAssetMetadata).toBeCalledTimes(1);
+    expect(gatsbyUtilsMocks.reporter.verbose).toBeCalledTimes(1);
+    // gatsby-plugin-image -> generateImageData should be called for both
     expect(generateImageData).toHaveBeenNthCalledWith(2, {
       filename: 'cloud-name>>>public-id',
       generateImageSource: _generateCloudinaryAssetSource,
@@ -129,7 +137,8 @@ describe('resolveCloudinaryAssetData', () => {
   });
 
   it('fetches and adds correct "blurred" placeholder', async () => {
-    await resolveCloudinaryAssetData(source, { placeholder: 'blurred' });
+    const args = { placeholder: 'blurred' };
+    await resolveCloudinaryAssetData(sourceWithMetadata, args, context, info);
 
     expect(getLowResolutionImageURL).toBeCalledTimes(1);
     expect(getUrlAsBase64Image).toBeCalledTimes(1);
@@ -149,7 +158,8 @@ describe('resolveCloudinaryAssetData', () => {
   });
 
   it('fetches and adds correct "tracedSVG" placeholder', async () => {
-    await resolveCloudinaryAssetData(source, { placeholder: 'tracedSVG' });
+    const args = { placeholder: 'tracedSVG' };
+    await resolveCloudinaryAssetData(sourceWithMetadata, args, context, info);
 
     expect(getAssetAsTracedSvg).toBeCalledTimes(1);
     expect(generateImageData).toHaveBeenCalledWith({
@@ -167,6 +177,22 @@ describe('resolveCloudinaryAssetData', () => {
     });
   });
 
+  describe('when missing required data', () => {
+    it('calls reporter.error and returns null', async () => {
+      const source = {};
+      const args = {};
+      const result = await resolveCloudinaryAssetData(
+        source,
+        args,
+        context,
+        info
+      );
+      expect(generateImageData).toBeCalledTimes(0);
+      expect(gatsbyUtilsMocks.reporter.error).toBeCalledTimes(1);
+      expect(result).toBe(null);
+    });
+  });
+
   describe('when error fetching asset data', () => {
     beforeEach(() => {
       getAssetMetadata.mockImplementation(() => {
@@ -185,17 +211,23 @@ describe('resolveCloudinaryAssetData', () => {
       jest.clearAllMocks();
     });
 
-    it('reporter.panic on fetching metdata error', async () => {
-      await resolveCloudinaryAssetData(
-        { publicId: 'public-id', cloudName: 'cloud-name' },
-        {}
+    it('calls reporter.error on fetching metadata error and returns null', async () => {
+      const args = {};
+      const result = await resolveCloudinaryAssetData(
+        sourceWithoutMeta,
+        args,
+        context,
+        info
       );
       expect(getAssetMetadata).toBeCalledTimes(1);
-      expect(gatsbyUtilsMocks.reporter.panic).toBeCalledTimes(1);
+      expect(generateImageData).toBeCalledTimes(0);
+      expect(gatsbyUtilsMocks.reporter.error).toBeCalledTimes(1);
+      expect(result).toBe(null);
     });
 
-    it('reporter.errror on fetching blurred placeholder error', async () => {
-      await resolveCloudinaryAssetData(source, { placeholder: 'blurred' });
+    it('calls reporter.error on fetching blurred placeholder error', async () => {
+      const args = { placeholder: 'blurred' };
+      await resolveCloudinaryAssetData(sourceWithMetadata, args, context, info);
       expect(getLowResolutionImageURL).toBeCalledTimes(1);
       expect(getUrlAsBase64Image).toBeCalledTimes(1);
       expect(gatsbyUtilsMocks.reporter.error).toBeCalledTimes(1);
@@ -214,8 +246,9 @@ describe('resolveCloudinaryAssetData', () => {
       });
     });
 
-    it('reporter.errror on fetching tracedSVG placeholder error', async () => {
-      await resolveCloudinaryAssetData(source, { placeholder: 'tracedSVG' });
+    it('calls reporter.error on fetching tracedSVG placeholder error', async () => {
+      const args = { placeholder: 'tracedSVG' };
+      await resolveCloudinaryAssetData(sourceWithMetadata, args, context, info);
       expect(getAssetAsTracedSvg).toBeCalledTimes(1);
       expect(gatsbyUtilsMocks.reporter.error).toBeCalledTimes(1);
       expect(generateImageData).toHaveBeenCalledWith({