From 9429899d7fe89d20a9f7182cc99cb2fdf1a20a79 Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Thu, 28 Mar 2019 23:32:07 +0100 Subject: [PATCH] fix(gatsby-plugin-sharp): should fix cannot read bitmap of undefined --- .../src/__tests__/trace-svg.js | 217 ++++++++++++++++++ .../gatsby-plugin-sharp/src/gatsby-node.js | 4 +- packages/gatsby-plugin-sharp/src/index.js | 188 +-------------- .../gatsby-plugin-sharp/src/plugin-options.js | 80 +++++++ .../gatsby-plugin-sharp/src/report-error.js | 12 + packages/gatsby-plugin-sharp/src/trace-svg.js | 136 +++++++++++ 6 files changed, 459 insertions(+), 178 deletions(-) create mode 100644 packages/gatsby-plugin-sharp/src/__tests__/trace-svg.js create mode 100644 packages/gatsby-plugin-sharp/src/plugin-options.js create mode 100644 packages/gatsby-plugin-sharp/src/report-error.js create mode 100644 packages/gatsby-plugin-sharp/src/trace-svg.js diff --git a/packages/gatsby-plugin-sharp/src/__tests__/trace-svg.js b/packages/gatsby-plugin-sharp/src/__tests__/trace-svg.js new file mode 100644 index 0000000000000..11ebdfa6fdb26 --- /dev/null +++ b/packages/gatsby-plugin-sharp/src/__tests__/trace-svg.js @@ -0,0 +1,217 @@ +jest.mock(`sharp`, () => { + const sharp = path => { + const pipeline = { + rotate: () => pipeline, + resize: () => pipeline, + png: () => pipeline, + jpeg: () => pipeline, + toFile: (_, cb) => cb(), + } + return pipeline + } + + sharp.simd = () => {} + + return sharp +}) + +jest.mock(`potrace`, () => { + return { + trace: (_, _2, cb) => cb(null, ``), + Potrace: { + TURNPOLICY_MAJORITY: `wat`, + }, + } +}) + +const path = require(`path`) + +const traceSVGHelpers = require(`../trace-svg`) + +const notMemoizedtraceSVG = jest.spyOn(traceSVGHelpers, `notMemoizedtraceSVG`) +const notMemoizedPrepareTraceSVGInputFile = jest.spyOn( + traceSVGHelpers, + `notMemoizedPrepareTraceSVGInputFile` +) +// note that we started spying on not memoized functions first +// now we recreate memoized functions that will use function we just started +// spying on +traceSVGHelpers.createMemoizedFunctions() +const memoizedTraceSVG = jest.spyOn(traceSVGHelpers, `memoizedTraceSVG`) +const memoizedPrepareTraceSVGInputFile = jest.spyOn( + traceSVGHelpers, + `memoizedPrepareTraceSVGInputFile` +) + +const { traceSVG } = require(`../`) + +function getFileObject(absolutePath, name = path.parse(absolutePath).name) { + return { + id: `${absolutePath} absPath of file`, + name: name, + absolutePath, + extension: `png`, + internal: { + contentDigest: `1234`, + }, + } +} + +describe(`traceSVG memoization`, () => { + const file = getFileObject(path.join(__dirname, `images/test.png`)) + const copyOfFile = getFileObject(path.join(__dirname, `images/test-copy.png`)) + const differentFile = getFileObject( + path.join(__dirname, `images/different.png`) + ) + differentFile.internal.contentDigest = `4321` + + beforeEach(() => { + traceSVGHelpers.clearMemoizeCaches() + memoizedTraceSVG.mockClear() + notMemoizedtraceSVG.mockClear() + memoizedPrepareTraceSVGInputFile.mockClear() + notMemoizedPrepareTraceSVGInputFile.mockClear() + }) + + it(`Baseline`, async () => { + await traceSVG({ + file, + }) + + expect(memoizedTraceSVG).toBeCalledTimes(1) + expect(notMemoizedtraceSVG).toBeCalledTimes(1) + expect(memoizedPrepareTraceSVGInputFile).toBeCalledTimes(1) + expect(notMemoizedPrepareTraceSVGInputFile).toBeCalledTimes(1) + }) + + it(`Is memoizing results for same args`, async () => { + await traceSVG({ + file, + }) + + await traceSVG({ + file, + }) + + expect(memoizedTraceSVG).toBeCalledTimes(2) + expect(notMemoizedtraceSVG).toBeCalledTimes(1) + expect(memoizedPrepareTraceSVGInputFile).toBeCalledTimes(1) + expect(notMemoizedPrepareTraceSVGInputFile).toBeCalledTimes(1) + }) + + it(`Is calling functions with same input file when params change`, async () => { + await traceSVG({ + file, + args: { + color: `red`, + }, + fileArgs: { + width: 400, + }, + }) + await traceSVG({ + file, + args: { + color: `blue`, + }, + fileArgs: { + width: 400, + }, + }) + await traceSVG({ + file, + args: { + color: `red`, + }, + fileArgs: { + width: 200, + }, + }) + await traceSVG({ + file, + args: { + color: `blue`, + }, + fileArgs: { + width: 200, + }, + }) + await traceSVG({ + file: differentFile, + args: { + color: `red`, + }, + fileArgs: { + width: 400, + }, + }) + + expect(memoizedTraceSVG).toBeCalledTimes(5) + expect(notMemoizedtraceSVG).toBeCalledTimes(5) + expect(memoizedPrepareTraceSVGInputFile).toBeCalledTimes(5) + // trace svg should be actually created just 3 times + // because it's affected just by `fileArgs`, and not `args` + // this makes sure we don't try to write to same input file multiple times + expect(notMemoizedPrepareTraceSVGInputFile).toBeCalledTimes(3) + expect(notMemoizedPrepareTraceSVGInputFile).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + file, + options: expect.objectContaining({ + width: 400, + }), + }) + ) + expect(notMemoizedPrepareTraceSVGInputFile).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + file, + options: expect.objectContaining({ + width: 200, + }), + }) + ) + expect(notMemoizedPrepareTraceSVGInputFile).toHaveBeenNthCalledWith( + 3, + expect.objectContaining({ + file: differentFile, + options: expect.objectContaining({ + width: 400, + }), + }) + ) + + const usedTmpFilePaths = notMemoizedPrepareTraceSVGInputFile.mock.calls.map( + args => args[0].tmpFilePath + ) + + // tmpFilePath was always unique + expect(usedTmpFilePaths.length).toBe(new Set(usedTmpFilePaths).size) + }) + + it(`Use memoized results for file copies`, async () => { + await traceSVG({ + file, + args: { + color: `red`, + }, + fileArgs: { + width: 400, + }, + }) + await traceSVG({ + file: copyOfFile, + args: { + color: `red`, + }, + fileArgs: { + width: 400, + }, + }) + + expect(memoizedTraceSVG).toBeCalledTimes(2) + expect(notMemoizedtraceSVG).toBeCalledTimes(1) + expect(memoizedPrepareTraceSVGInputFile).toBeCalledTimes(1) + expect(notMemoizedPrepareTraceSVGInputFile).toBeCalledTimes(1) + }) +}) diff --git a/packages/gatsby-plugin-sharp/src/gatsby-node.js b/packages/gatsby-plugin-sharp/src/gatsby-node.js index 8e2abdfd08145..20a15493c5a44 100644 --- a/packages/gatsby-plugin-sharp/src/gatsby-node.js +++ b/packages/gatsby-plugin-sharp/src/gatsby-node.js @@ -1,9 +1,11 @@ const { setBoundActionCreators, - setPluginOptions, // queue: jobQueue, // reportError, } = require(`./index`) + +const { setPluginOptions } = require(`./plugin-options`) + // const { scheduleJob } = require(`./scheduler`) // let normalizedOptions = {} diff --git a/packages/gatsby-plugin-sharp/src/index.js b/packages/gatsby-plugin-sharp/src/index.js index b4148b2f902ae..89ae6e393b83f 100644 --- a/packages/gatsby-plugin-sharp/src/index.js +++ b/packages/gatsby-plugin-sharp/src/index.js @@ -18,14 +18,18 @@ try { } const sharp = require(`sharp`) -const crypto = require(`crypto`) + const imageSize = require(`probe-image-size`) -const { promisify } = require(`bluebird`) + const _ = require(`lodash`) const fs = require(`fs-extra`) const path = require(`path`) + const { scheduleJob } = require(`./scheduler`) const { createArgsDigest } = require(`./process-file`) +const { reportError } = require(`./report-error`) +const { getPluginOptions, healOptions } = require(`./plugin-options`) +const { memoizedTraceSVG } = require(`./trace-svg`) const imageSizeCache = new Map() const getImageSize = file => { @@ -59,95 +63,8 @@ exports.setBoundActionCreators = actions => { const queue = new Map() exports.queue = queue -/// Plugin options are loaded onPreInit in gatsby-node -const pluginDefaults = { - forceBase64Format: false, - useMozJpeg: process.env.GATSBY_JPEG_ENCODER === `MOZJPEG`, - stripMetadata: true, - lazyImageGeneration: true, - defaultQuality: 50, -} - -const generalArgs = { - quality: 50, - jpegProgressive: true, - pngCompressionLevel: 9, - // default is 4 (https://github.com/kornelski/pngquant/blob/4219956d5e080be7905b5581314d913d20896934/rust/bin.rs#L61) - pngCompressionSpeed: 4, - base64: true, - grayscale: false, - duotone: false, - pathPrefix: ``, - toFormat: ``, - toFormatBase64: ``, - sizeByPixelDensity: false, -} - -let pluginOptions = Object.assign({}, pluginDefaults) -exports.setPluginOptions = opts => { - pluginOptions = Object.assign({}, pluginOptions, opts) - generalArgs.quality = pluginOptions.defaultQuality - - return pluginOptions -} - -const reportError = (message, err, reporter) => { - if (reporter) { - reporter.error(message, err) - } else { - console.error(message, err) - } - - if (process.env.gatsby_executing_command === `build`) { - process.exit(1) - } -} -exports.reportError = reportError - -const healOptions = ( - { defaultQuality: quality }, - args, - fileExtension, - defaultArgs = {} -) => { - let options = _.defaults({}, args, { quality }, defaultArgs, generalArgs) - options.quality = parseInt(options.quality, 10) - options.pngCompressionLevel = parseInt(options.pngCompressionLevel, 10) - options.pngCompressionSpeed = parseInt(options.pngCompressionSpeed, 10) - options.toFormat = options.toFormat.toLowerCase() - options.toFormatBase64 = options.toFormatBase64.toLowerCase() - - // when toFormat is not set we set it based on fileExtension - if (options.toFormat === ``) { - options.toFormat = fileExtension.toLowerCase() - - if (fileExtension === `jpeg`) { - options.toFormat = `jpg` - } - } - - // only set width to 400 if neither width nor height is passed - if (options.width === undefined && options.height === undefined) { - options.width = 400 - } else if (options.width !== undefined) { - options.width = parseInt(options.width, 10) - } else if (options.height !== undefined) { - options.height = parseInt(options.height, 10) - } - - // only set maxWidth to 800 if neither maxWidth nor maxHeight is passed - if (options.maxWidth === undefined && options.maxHeight === undefined) { - options.maxWidth = 800 - } else if (options.maxWidth !== undefined) { - options.maxWidth = parseInt(options.maxWidth, 10) - } else if (options.maxHeight !== undefined) { - options.maxHeight = parseInt(options.maxHeight, 10) - } - - return options -} - function queueImageResizing({ file, args = {}, reporter }) { + const pluginOptions = getPluginOptions() const options = healOptions(pluginOptions, args, file.extension) if (!options.toFormat) { options.toFormat = file.extension @@ -238,8 +155,9 @@ function queueImageResizing({ file, args = {}, reporter }) { } // A value in pixels(Int) -const defaultBase64Width = () => pluginOptions.base64Width || 20 +const defaultBase64Width = () => getPluginOptions().base64Width || 20 async function generateBase64({ file, args, reporter }) { + const pluginOptions = getPluginOptions() const options = healOptions(pluginOptions, args, file.extension, { width: defaultBase64Width(), }) @@ -344,7 +262,7 @@ async function getTracedSVG(options, file) { } async function fluid({ file, args = {}, reporter, cache }) { - const options = healOptions(pluginOptions, args, file.extension) + const options = healOptions(getPluginOptions(), args, file.extension) // Account for images with a high pixel density. We assume that these types of // images are intended to be displayed at their native resolution. let metadata @@ -526,7 +444,7 @@ async function fluid({ file, args = {}, reporter, cache }) { } async function fixed({ file, args = {}, reporter, cache }) { - const options = healOptions(pluginOptions, args, file.extension) + const options = healOptions(getPluginOptions(), args, file.extension) // if no width is passed, we need to resize the image based on the passed height const fixedDimension = options.width === undefined ? `height` : `width` @@ -639,94 +557,10 @@ async function fixed({ file, args = {}, reporter, cache }) { } } -async function notMemoizedtraceSVG({ file, args, fileArgs, reporter }) { - const potrace = require(`potrace`) - const svgToMiniDataURI = require(`mini-svg-data-uri`) - const trace = promisify(potrace.trace) - const defaultArgs = { - color: `lightgray`, - optTolerance: 0.4, - turdSize: 100, - turnPolicy: potrace.Potrace.TURNPOLICY_MAJORITY, - } - const optionsSVG = _.defaults(args, defaultArgs) - const options = healOptions(pluginOptions, fileArgs, file.extension) - let pipeline - try { - pipeline = sharp(file.absolutePath).rotate() - } catch (err) { - reportError(`Failed to process image ${file.absolutePath}`, err, reporter) - return null - } - - pipeline - .resize(options.width, options.height, { - position: options.cropFocus, - }) - .png({ - compressionLevel: options.pngCompressionLevel, - adaptiveFiltering: false, - force: args.toFormat === `png`, - }) - .jpeg({ - quality: options.quality, - progressive: options.jpegProgressive, - force: args.toFormat === `jpg`, - }) - - // grayscale - if (options.grayscale) { - pipeline = pipeline.grayscale() - } - - // rotate - if (options.rotate && options.rotate !== 0) { - pipeline = pipeline.rotate(options.rotate) - } - - // duotone - if (options.duotone) { - pipeline = await duotone( - options.duotone, - args.toFormat || file.extension, - pipeline - ) - } - - const tmpDir = require(`os`).tmpdir() - const tmpFilePath = `${tmpDir}/${file.internal.contentDigest}-${ - file.name - }-${crypto - .createHash(`md5`) - .update(JSON.stringify(fileArgs)) - .digest(`hex`)}.${file.extension}` - - await new Promise(resolve => - pipeline.toFile(tmpFilePath, (err, info) => { - resolve() - }) - ) - - return trace(tmpFilePath, optionsSVG) - .then(optimize) - .then(svgToMiniDataURI) -} - -const memoizedTraceSVG = _.memoize( - notMemoizedtraceSVG, - ({ file, args }) => `${file.absolutePath}${JSON.stringify(args)}` -) - async function traceSVG(args) { return await memoizedTraceSVG(args) } -const optimize = svg => { - const SVGO = require(`svgo`) - const svgo = new SVGO({ multipass: true, floatPrecision: 0 }) - return svgo.optimize(svg).then(({ data }) => data) -} - function toArray(buf) { var arr = new Array(buf.length) diff --git a/packages/gatsby-plugin-sharp/src/plugin-options.js b/packages/gatsby-plugin-sharp/src/plugin-options.js new file mode 100644 index 0000000000000..61ca0de5f7c4c --- /dev/null +++ b/packages/gatsby-plugin-sharp/src/plugin-options.js @@ -0,0 +1,80 @@ +const _ = require(`lodash`) + +/// Plugin options are loaded onPreBootstrap in gatsby-node +const pluginDefaults = { + forceBase64Format: false, + useMozJpeg: process.env.GATSBY_JPEG_ENCODER === `MOZJPEG`, + stripMetadata: true, + lazyImageGeneration: true, + defaultQuality: 50, +} + +const generalArgs = { + quality: 50, + jpegProgressive: true, + pngCompressionLevel: 9, + // default is 4 (https://github.com/kornelski/pngquant/blob/4219956d5e080be7905b5581314d913d20896934/rust/bin.rs#L61) + pngCompressionSpeed: 4, + base64: true, + grayscale: false, + duotone: false, + pathPrefix: ``, + toFormat: ``, + toFormatBase64: ``, + sizeByPixelDensity: false, +} + +let pluginOptions = Object.assign({}, pluginDefaults) +exports.setPluginOptions = opts => { + pluginOptions = Object.assign({}, pluginOptions, opts) + generalArgs.quality = pluginOptions.defaultQuality + + return pluginOptions +} + +exports.getPluginOptions = () => pluginOptions + +const healOptions = ( + { defaultQuality: quality }, + args, + fileExtension, + defaultArgs = {} +) => { + let options = _.defaults({}, args, { quality }, defaultArgs, generalArgs) + options.quality = parseInt(options.quality, 10) + options.pngCompressionLevel = parseInt(options.pngCompressionLevel, 10) + options.pngCompressionSpeed = parseInt(options.pngCompressionSpeed, 10) + options.toFormat = options.toFormat.toLowerCase() + options.toFormatBase64 = options.toFormatBase64.toLowerCase() + + // when toFormat is not set we set it based on fileExtension + if (options.toFormat === ``) { + options.toFormat = fileExtension.toLowerCase() + + if (fileExtension === `jpeg`) { + options.toFormat = `jpg` + } + } + + // only set width to 400 if neither width nor height is passed + if (options.width === undefined && options.height === undefined) { + options.width = 400 + } else if (options.width !== undefined) { + options.width = parseInt(options.width, 10) + } else if (options.height !== undefined) { + options.height = parseInt(options.height, 10) + } + + // only set maxWidth to 800 if neither maxWidth nor maxHeight is passed + if (options.maxWidth === undefined && options.maxHeight === undefined) { + options.maxWidth = 800 + } else if (options.maxWidth !== undefined) { + options.maxWidth = parseInt(options.maxWidth, 10) + } else if (options.maxHeight !== undefined) { + options.maxHeight = parseInt(options.maxHeight, 10) + } + + return options +} + +exports.healOptions = healOptions diff --git a/packages/gatsby-plugin-sharp/src/report-error.js b/packages/gatsby-plugin-sharp/src/report-error.js new file mode 100644 index 0000000000000..b1c1c3f0a516e --- /dev/null +++ b/packages/gatsby-plugin-sharp/src/report-error.js @@ -0,0 +1,12 @@ +const reportError = (message, err, reporter) => { + if (reporter) { + reporter.error(message, err) + } else { + console.error(message, err) + } + + if (process.env.gatsby_executing_command === `build`) { + process.exit(1) + } +} +exports.reportError = reportError diff --git a/packages/gatsby-plugin-sharp/src/trace-svg.js b/packages/gatsby-plugin-sharp/src/trace-svg.js new file mode 100644 index 0000000000000..14d722123e8a9 --- /dev/null +++ b/packages/gatsby-plugin-sharp/src/trace-svg.js @@ -0,0 +1,136 @@ +const { promisify } = require(`bluebird`) +const crypto = require(`crypto`) +const _ = require(`lodash`) +const tmpDir = require(`os`).tmpdir() +const sharp = require(`sharp`) + +const duotone = require(`./duotone`) +const { getPluginOptions, healOptions } = require(`./plugin-options`) +const { reportError } = require(`./report-error`) + +exports.notMemoizedPrepareTraceSVGInputFile = async ({ + file, + options, + tmpFilePath, + reporter, +}) => { + let pipeline + try { + pipeline = sharp(file.absolutePath).rotate() + } catch (err) { + reportError(`Failed to process image ${file.absolutePath}`, err, reporter) + return + } + + pipeline + .resize(options.width, options.height, { + position: options.cropFocus, + }) + .png({ + compressionLevel: options.pngCompressionLevel, + adaptiveFiltering: false, + force: options.toFormat === `png`, + }) + .jpeg({ + quality: options.quality, + progressive: options.jpegProgressive, + force: options.toFormat === `jpg`, + }) + + // grayscale + if (options.grayscale) { + pipeline = pipeline.grayscale() + } + + // rotate + if (options.rotate && options.rotate !== 0) { + pipeline = pipeline.rotate(options.rotate) + } + + // duotone + if (options.duotone) { + pipeline = await duotone(options.duotone, options.toFormat, pipeline) + } + + await new Promise((resolve, reject) => + pipeline.toFile(tmpFilePath, err => { + if (err) { + return reject(err) + } + return resolve() + }) + ) +} + +const optimize = svg => { + const SVGO = require(`svgo`) + const svgo = new SVGO({ multipass: true, floatPrecision: 0 }) + return svgo.optimize(svg).then(({ data }) => data) +} + +exports.notMemoizedtraceSVG = async ({ file, args, fileArgs, reporter }) => { + const options = healOptions(getPluginOptions(), fileArgs, file.extension) + + const tmpFilePath = `${tmpDir}/${file.internal.contentDigest}-${ + file.name + }-${crypto + .createHash(`md5`) + .update(JSON.stringify(options)) + .digest(`hex`)}.${file.extension}` + + try { + await exports.memoizedPrepareTraceSVGInputFile({ + tmpFilePath, + file, + options, + reporter, + }) + + const svgToMiniDataURI = require(`mini-svg-data-uri`) + const potrace = require(`potrace`) + const trace = promisify(potrace.trace) + + const defaultArgs = { + color: `lightgray`, + optTolerance: 0.4, + turdSize: 100, + turnPolicy: potrace.Potrace.TURNPOLICY_MAJORITY, + } + + const optionsSVG = _.defaults(args, defaultArgs) + + return trace(tmpFilePath, optionsSVG) + .then(optimize) + .then(svgToMiniDataURI) + } catch (e) { + throw e + } +} + +let memoizedPrepareTraceSVGInputFile, memoizedTraceSVG +const createMemoizedFunctions = () => { + exports.memoizedPrepareTraceSVGInputFile = memoizedPrepareTraceSVGInputFile = _.memoize( + exports.notMemoizedPrepareTraceSVGInputFile, + ({ tmpFilePath }) => tmpFilePath + ) + + exports.memoizedTraceSVG = memoizedTraceSVG = _.memoize( + exports.notMemoizedtraceSVG, + ({ file, args, fileArgs }) => + `${file.internal.contentDigest}${JSON.stringify(args)}${JSON.stringify( + fileArgs + )}` + ) +} + +// This is very hacky, but memozied function are pretty tricky to spy on +// in tests ;( +createMemoizedFunctions() +exports.createMemoizedFunctions = () => { + createMemoizedFunctions() +} + +exports.clearMemoizeCaches = () => { + memoizedTraceSVG.cache.clear() + memoizedPrepareTraceSVGInputFile.cache.clear() +}