fix(gatsby-plugin-sharp): fix progressbar & caching layer (#19717)

gatsbyjs · Nov 24, 2019 · 7f9d5bb · 7f9d5bb
1 parent 9eb2e8e
commit 7f9d5bb
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 13 deletions.
diff --git a/packages/gatsby-plugin-sharp/src/__tests__/index.js b/packages/gatsby-plugin-sharp/src/__tests__/index.js
@@ -9,8 +9,14 @@ jest.mock(`async/queue`, () => () => {
   }
 })
 
+const { scheduleJob } = require(`../scheduler`)
+scheduleJob.mockReturnValue(Promise.resolve())
 fs.ensureDirSync = jest.fn()
 fs.existsSync = jest.fn().mockReturnValue(false)
+let isolatedQueueImageResizing
+jest.isolateModules(() => {
+  isolatedQueueImageResizing = require(`../index`).queueImageResizing
+})
 
 const {
   base64,
@@ -20,8 +26,6 @@ const {
   getImageSize,
   stats,
 } = require(`../`)
-const { scheduleJob } = require(`../scheduler`)
-scheduleJob.mockResolvedValue(Promise.resolve())
 
 jest.mock(`gatsby-cli/lib/reporter`, () => {
   return {
@@ -56,6 +60,12 @@ describe(`gatsby-plugin-sharp`, () => {
   }
 
   describe(`queueImageResizing`, () => {
+    beforeEach(() => {
+      scheduleJob.mockClear()
+      fs.existsSync.mockReset()
+      fs.existsSync.mockReturnValue(false)
+    })
+
     it(`should round height when auto-calculated`, () => {
       // Resize 144-density.png (281x136) with a 3px width
       const result = queueImageResizing({
@@ -93,7 +103,6 @@ describe(`gatsby-plugin-sharp`, () => {
 
     // re-enable when image processing on demand is implemented
     it.skip(`should process immediately when asked`, async () => {
-      scheduleJob.mockClear()
       const result = queueImageResizing({
         file: getFileObject(path.join(__dirname, `images/144-density.png`)),
         args: { width: 3 },
@@ -105,7 +114,6 @@ describe(`gatsby-plugin-sharp`, () => {
     })
 
     it(`Shouldn't schedule a job when outputFile already exists`, async () => {
-      scheduleJob.mockClear()
       fs.existsSync.mockReturnValue(true)
 
       const result = queueImageResizing({
@@ -118,6 +126,21 @@ describe(`gatsby-plugin-sharp`, () => {
       expect(fs.existsSync).toHaveBeenCalledWith(result.absolutePath)
       expect(scheduleJob).not.toHaveBeenCalled()
     })
+
+    it(`Shouldn't schedule a job when with same outputFile is already being queued`, async () => {
+      const result = isolatedQueueImageResizing({
+        file: getFileObject(path.join(__dirname, `images/144-density.png`)),
+        args: { width: 5 },
+      })
+      isolatedQueueImageResizing({
+        file: getFileObject(path.join(__dirname, `images/144-density.png`)),
+        args: { width: 5 },
+      })
+
+      await result.finishedPromise
+
+      expect(scheduleJob).toHaveBeenCalledTimes(1)
+    })
   })
 
   describe(`fluid`, () => {

diff --git a/packages/gatsby-plugin-sharp/src/__tests__/scheduler.js b/packages/gatsby-plugin-sharp/src/__tests__/scheduler.js
@@ -40,7 +40,7 @@ describe(`scheduler`, () => {
       },
       contentDigest: `digest`,
     }
-    await scheduleJob(job, boundActionCreators, {}, false, `value`)
+    await scheduleJob(job, boundActionCreators, `value`, {}, false)
 
     expect(createProgress).not.toHaveBeenCalled()
     expect(workerMock).toHaveBeenCalledWith([job.inputPath], job.outputDir, {
@@ -110,6 +110,7 @@ describe(`scheduler`, () => {
         },
         boundActionCreators,
         {},
+        {},
         false
       )
     } catch (err) {
@@ -147,7 +148,7 @@ describe(`scheduler`, () => {
       },
       contentDigest: `digest`,
     }
-    const job1Promise = scheduleJob(job1, boundActionCreators, {}, false)
+    const job1Promise = scheduleJob(job1, boundActionCreators, {}, {}, false)
 
     const job2 = {
       inputPath: `/test-image.jpg`,
@@ -158,7 +159,7 @@ describe(`scheduler`, () => {
       },
       contentDigest: `digest`,
     }
-    const job2Promise = scheduleJob(job2, boundActionCreators, {}, false)
+    const job2Promise = scheduleJob(job2, boundActionCreators, {}, {}, false)
 
     await Promise.all([job1Promise, job2Promise])
 

diff --git a/packages/gatsby-plugin-sharp/src/index.js b/packages/gatsby-plugin-sharp/src/index.js
@@ -40,6 +40,7 @@ exports.setBoundActionCreators = actions => {
   boundActionCreators = actions
 }
 
+const cachedOutputFiles = new Map()
 function queueImageResizing({ file, args = {}, reporter }) {
   const pluginOptions = getPluginOptions()
   const options = healOptions(pluginOptions, args, file.extension)
@@ -103,19 +104,29 @@ function queueImageResizing({ file, args = {}, reporter }) {
     outputPath: outputFilePath,
   }
 
+  const outputFile = path.join(job.outputDir, job.outputPath)
   let finishedPromise = Promise.resolve()
 
-  // Check if the output file already exists so we don't redo work.
+  if (cachedOutputFiles.has(outputFile)) {
+    finishedPromise = cachedOutputFiles.get(outputFile)
+  }
+
+  // Check if the output file already exists or already is being created.
   // TODO: Remove this when jobs api is stable, it will have a better check
-  const outputFile = path.join(job.outputDir, job.outputPath)
-  if (!fs.existsSync(outputFile)) {
+  if (!fs.existsSync(outputFile) && !cachedOutputFiles.has(outputFile)) {
     // schedule job immediately - this will be changed when image processing on demand is implemented
     finishedPromise = scheduleJob(
       job,
       boundActionCreators,
       pluginOptions,
       reporter
-    )
+    ).then(res => {
+      cachedOutputFiles.delete(outputFile)
+
+      return res
+    })
+
+    cachedOutputFiles.set(outputFile, finishedPromise)
   }
 
   return {

diff --git a/packages/gatsby-plugin-sharp/src/scheduler.js b/packages/gatsby-plugin-sharp/src/scheduler.js
@@ -60,9 +60,9 @@ const executeJobs = _.throttle(
 const scheduleJob = async (
   job,
   boundActionCreators,
+  pluginOptions,
   reporter,
-  reportStatus = true,
-  pluginOptions
+  reportStatus = true
 ) => {
   const isQueued = toProcess.has(job.inputPath)
   let scheduledPromise