Skip to content

Commit

Permalink
feat: allow invalid source data by making gatsbyImageData nullable (#218
Browse files Browse the repository at this point in the history 
)

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
  • Loading branch information
@raae
raae authored Dec 20, 2022
1 parent 15f0682 commit acf28f9
Show file tree
Hide file tree
Showing 6 changed files with 129 additions and 37 deletions.
1 change: 1 addition & 0 deletions demo/gatsby-config.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ module.exports = {
'CloudinaryAsset',
'ExistingDataExampleImage',
'ExistingDataNestedExampleImage',
'SomeBadImageData',
],
},
},
Expand Down
19 changes: 19 additions & 0 deletions demo/gatsby-node.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
28 changes: 28 additions & 0 deletions demo/src/pages/somebaddata.js
View file
Original file line number Diff line number Diff line change
@@ -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;
2 changes: 2 additions & 0 deletions plugin/gatsby-plugin-image/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
37 changes: 23 additions & 14 deletions plugin/gatsby-plugin-image/resolve-asset.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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('>>>');
Expand All @@ -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,
Expand All @@ -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;
}
}

Expand All @@ -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,
};

Expand All @@ -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}`
);
}

Expand Down
79 changes: 56 additions & 23 deletions plugin/gatsby-plugin-image/resolve-asset.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
},
};
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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);
Expand All @@ -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({
Expand All @@ -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(() => {
Expand All @@ -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);
Expand All @@ -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({
Expand Down

0 comments on commit acf28f9

Please sign in to comment.