fix(gatsby-plugin-sharp): hot fix for "Generating image thumbnails" p…

…rogress bar reporting duplicates that don't do actual processing (#20839)

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": "<PROJECT_ROOT>/public/static/1234",
  },
  Object {},
+ undefined,
  ],
  ],
  "results": Array [
@@ -945,6 +946,7 @@ exports[`gatsby-plugin-sharp queueImageResizing with createJob should round heig
  "outputDir": "<PROJECT_ROOT>/public/static/1234",
  },
  Object {},
+ undefined,
  ],
  ],
  "results": Array [

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))

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 <JobContentHash, TransformsCount> 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)
 }
 

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
 }

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)
  })
  })
 

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