diff --git a/packages/gatsby-plugin-sharp/src/__tests__/__snapshots__/index.js.snap b/packages/gatsby-plugin-sharp/src/__tests__/__snapshots__/index.js.snap index 428ff317372fe..b55492571a457 100644 --- a/packages/gatsby-plugin-sharp/src/__tests__/__snapshots__/index.js.snap +++ b/packages/gatsby-plugin-sharp/src/__tests__/__snapshots__/index.js.snap @@ -904,6 +904,7 @@ exports[`gatsby-plugin-sharp queueImageResizing with createJob file name works w "outputDir": "/public/static/1234", }, Object {}, + undefined, ], ], "results": Array [ @@ -945,6 +946,7 @@ exports[`gatsby-plugin-sharp queueImageResizing with createJob should round heig "outputDir": "/public/static/1234", }, Object {}, + undefined, ], ], "results": Array [ diff --git a/packages/gatsby-plugin-sharp/src/__tests__/utils.js b/packages/gatsby-plugin-sharp/src/__tests__/utils.js index 633988595d216..e861c5b757d9f 100644 --- a/packages/gatsby-plugin-sharp/src/__tests__/utils.js +++ b/packages/gatsby-plugin-sharp/src/__tests__/utils.js @@ -1,23 +1,28 @@ jest.mock(`gatsby-cli/lib/reporter`) jest.mock(`progress`) -const { createProgress } = require(`../utils`) +const { + createGatsbyProgressOrFallbackToExternalProgressBar, +} = require(`../utils`) const reporter = require(`gatsby-cli/lib/reporter`) const progress = require(`progress`) -describe(`createProgress`, () => { +describe(`createGatsbyProgressOrFallbackToExternalProgressBar`, () => { beforeEach(() => { progress.mockClear() }) it(`should use createProgress from gatsby-cli when available`, () => { - createProgress(`test`, reporter) + createGatsbyProgressOrFallbackToExternalProgressBar(`test`, reporter) expect(reporter.createProgress).toBeCalled() expect(progress).not.toBeCalled() }) it(`should fallback to a local implementation when createProgress does not exists on reporter`, () => { reporter.createProgress = null - const bar = createProgress(`test`, reporter) + const bar = createGatsbyProgressOrFallbackToExternalProgressBar( + `test`, + reporter + ) expect(progress).toHaveBeenCalledTimes(1) expect(bar).toHaveProperty(`start`, expect.any(Function)) expect(bar).toHaveProperty(`tick`, expect.any(Function)) @@ -26,7 +31,7 @@ describe(`createProgress`, () => { }) it(`should fallback to a local implementation when no reporter is present`, () => { - const bar = createProgress(`test`) + const bar = createGatsbyProgressOrFallbackToExternalProgressBar(`test`) expect(progress).toHaveBeenCalledTimes(1) expect(bar).toHaveProperty(`start`, expect.any(Function)) expect(bar).toHaveProperty(`tick`, expect.any(Function)) diff --git a/packages/gatsby-plugin-sharp/src/gatsby-node.js b/packages/gatsby-plugin-sharp/src/gatsby-node.js index 1223cd872c5c3..dddb7bcfe155f 100644 --- a/packages/gatsby-plugin-sharp/src/gatsby-node.js +++ b/packages/gatsby-plugin-sharp/src/gatsby-node.js @@ -1,9 +1,9 @@ const { setBoundActionCreators, - getProgressBar, // queue: jobQueue, // reportError, } = require(`./index`) +const { getProgressBar, createOrGetProgressBar } = require(`./utils`) const { setPluginOptions } = require(`./plugin-options`) @@ -63,9 +63,47 @@ const finishProgressBar = () => { exports.onPostBuild = () => finishProgressBar() exports.onCreateDevServer = () => finishProgressBar() -exports.onPreBootstrap = ({ actions }, pluginOptions) => { +exports.onPreBootstrap = ({ actions, emitter, reporter }, pluginOptions) => { setBoundActionCreators(actions) setPluginOptions(pluginOptions) + + // below is a hack / hot fix for confusing progress bar behaviour + // that doesn't recognize duplicate jobs, as it's now + // in gatsby core internals (if `createJobV2` is available) + // we should remove this or make this code path not hit + // (we should never use emitter in plugins) + // as soon as possible (possibly by moving progress bar handling + // inside jobs-manager in core) + + if (emitter) { + // track how many image transformation each job has + // END_JOB_V2 doesn't contain that information + // so we store it in map + // when job is created. Then retrieve that when job finishes + // and remove that entry from the map. + const imageCountInJobsMap = new Map() + + emitter.on(`CREATE_JOB_V2`, action => { + if (action.plugin.name === `gatsby-plugin-sharp`) { + const job = action.payload.job + const imageCount = job.args.operations.length + imageCountInJobsMap.set(job.contentDigest, imageCount) + const progress = createOrGetProgressBar(reporter) + progress.addImageToProcess(imageCount) + } + }) + + emitter.on(`END_JOB_V2`, action => { + if (action.plugin.name === `gatsby-plugin-sharp`) { + const jobContentDigest = action.payload.jobContentDigest + const imageCount = imageCountInJobsMap.get(jobContentDigest) + const progress = createOrGetProgressBar(reporter) + progress.tick(imageCount) + imageCountInJobsMap.delete(jobContentDigest) + } + }) + } + // normalizedOptions = setPluginOptions(pluginOptions) } diff --git a/packages/gatsby-plugin-sharp/src/index.js b/packages/gatsby-plugin-sharp/src/index.js index e638ecb69d234..2ae2ab9aa04dd 100644 --- a/packages/gatsby-plugin-sharp/src/index.js +++ b/packages/gatsby-plugin-sharp/src/index.js @@ -18,7 +18,6 @@ const { const { memoizedTraceSVG, notMemoizedtraceSVG } = require(`./trace-svg`) const duotone = require(`./duotone`) const { IMAGE_PROCESSING_JOB_NAME } = require(`./gatsby-worker`) -const { createProgress } = require(`./utils`) const imageSizeCache = new Map() const getImageSize = file => { @@ -36,46 +35,6 @@ const getImageSize = file => { } } -let progressBar -let pendingImagesCounter = 0 -let firstPass = true -const createOrGetProgressBar = reporter => { - if (!progressBar) { - progressBar = createProgress(`Generating image thumbnails`, reporter) - - const originalDoneFn = progressBar.done - - // TODO this logic should be moved to the reporter. - // when done is called we remove the progressbar instance and reset all the things - // this will be called onPostBuild or when devserver is created - progressBar.done = () => { - originalDoneFn.call(progressBar) - progressBar = null - pendingImagesCounter = 0 - } - - // when we create a progressBar for the second time so when .done() has been called before - // we create a modified tick function that automatically stops the progressbar when total is reached - // this is used for development as we're watching for changes - if (!firstPass) { - let progressBarCurrentValue = 0 - const originalTickFn = progressBar.tick - progressBar.tick = (ticks = 1) => { - originalTickFn.call(progressBar, ticks) - progressBarCurrentValue += ticks - - if (progressBarCurrentValue === pendingImagesCounter) { - progressBar.done() - } - } - } - firstPass = false - } - - return progressBar -} -exports.getProgressBar = () => progressBar - // Bound action creators should be set when passed to onPreInit in gatsby-node. // ** It is NOT safe to just directly require the gatsby module **. // There is no guarantee that the module resolved is the module executing! @@ -148,16 +107,6 @@ function prepareQueue({ file, args }) { } function createJob(job, { reporter }) { - const progressBar = createOrGetProgressBar(reporter) - - if (pendingImagesCounter === 0) { - progressBar.start() - } - - const transformsCount = job.args.operations.length - pendingImagesCounter += transformsCount - progressBar.total = pendingImagesCounter - // Jobs can be duplicates and usually are long running tasks. // Because of that we shouldn't use async/await and instead opt to use // .then() /.catch() handlers, because this allows V8 to release @@ -169,16 +118,12 @@ function createJob(job, { reporter }) { if (boundActionCreators.createJobV2) { promise = boundActionCreators.createJobV2(job) } else { - promise = scheduleJob(job, boundActionCreators) + promise = scheduleJob(job, boundActionCreators, reporter) } - promise - .catch(err => { - reporter.panic(err) - }) - .then(() => { - progressBar.tick(transformsCount) - }) + promise.catch(err => { + reporter.panic(err) + }) return promise } diff --git a/packages/gatsby-plugin-sharp/src/scheduler.js b/packages/gatsby-plugin-sharp/src/scheduler.js index e9f1b6fdec438..4180d2bc81815 100644 --- a/packages/gatsby-plugin-sharp/src/scheduler.js +++ b/packages/gatsby-plugin-sharp/src/scheduler.js @@ -4,6 +4,7 @@ const fs = require(`fs-extra`) const got = require(`got`) const { createContentDigest } = require(`gatsby-core-utils`) const worker = require(`./gatsby-worker`) +const { createOrGetProgressBar } = require(`./utils`) const processImages = async (jobId, job, boundActionCreators) => { try { @@ -16,7 +17,7 @@ const processImages = async (jobId, job, boundActionCreators) => { } const jobsInFlight = new Map() -const scheduleJob = async (job, boundActionCreators) => { +const scheduleJob = async (job, boundActionCreators, reporter) => { const inputPaths = job.inputPaths.filter( inputPath => !fs.existsSync(path.join(job.outputDir, inputPath)) ) @@ -70,12 +71,16 @@ const scheduleJob = async (job, boundActionCreators) => { { name: `gatsby-plugin-sharp` } ) + const progressBar = createOrGetProgressBar(reporter) + const transformsCount = job.args.operations.length + progressBar.addImageToProcess(transformsCount) + const promise = new Promise((resolve, reject) => { setImmediate(() => { - processImages(jobId, convertedJob, boundActionCreators).then( - resolve, - reject - ) + processImages(jobId, convertedJob, boundActionCreators).then(result => { + progressBar.tick(transformsCount) + resolve(result) + }, reject) }) }) diff --git a/packages/gatsby-plugin-sharp/src/utils.js b/packages/gatsby-plugin-sharp/src/utils.js index 52694aab05004..f6162a449c1cc 100644 --- a/packages/gatsby-plugin-sharp/src/utils.js +++ b/packages/gatsby-plugin-sharp/src/utils.js @@ -1,7 +1,10 @@ const ProgressBar = require(`progress`) // TODO remove in V3 -export function createProgress(message, reporter) { +function createGatsbyProgressOrFallbackToExternalProgressBar( + message, + reporter +) { if (reporter && reporter.createProgress) { return reporter.createProgress(message) } @@ -26,3 +29,57 @@ export function createProgress(message, reporter) { }, } } + +let progressBar +let pendingImagesCounter = 0 +let firstPass = true +const createOrGetProgressBar = reporter => { + if (!progressBar) { + progressBar = createGatsbyProgressOrFallbackToExternalProgressBar( + `Generating image thumbnails`, + reporter + ) + + const originalDoneFn = progressBar.done + + // TODO this logic should be moved to the reporter. + // when done is called we remove the progressbar instance and reset all the things + // this will be called onPostBuild or when devserver is created + progressBar.done = () => { + originalDoneFn.call(progressBar) + progressBar = null + pendingImagesCounter = 0 + } + + progressBar.addImageToProcess = imageCount => { + if (pendingImagesCounter === 0) { + progressBar.start() + } + pendingImagesCounter += imageCount + progressBar.total = pendingImagesCounter + } + + // when we create a progressBar for the second time so when .done() has been called before + // we create a modified tick function that automatically stops the progressbar when total is reached + // this is used for development as we're watching for changes + if (!firstPass) { + let progressBarCurrentValue = 0 + const originalTickFn = progressBar.tick + progressBar.tick = (ticks = 1) => { + originalTickFn.call(progressBar, ticks) + progressBarCurrentValue += ticks + + if (progressBarCurrentValue === pendingImagesCounter) { + progressBar.done() + } + } + } + firstPass = false + } + + return progressBar +} + +exports.createGatsbyProgressOrFallbackToExternalProgressBar = createGatsbyProgressOrFallbackToExternalProgressBar +exports.createOrGetProgressBar = createOrGetProgressBar +exports.getProgressBar = () => progressBar