fix: if preprocessor is async function and doesn't return a content then await donePromise (#3387)

anthony-redFox · johnjbarton · commit f91be246e760 · 2019-10-18T11:49:58.000-07:00
diff --git a/lib/preprocessor.js b/lib/preprocessor.js
@@ -24,7 +24,15 @@ function executeProcessor (process, file, content) {
       }
     }
   })
-  return process(content, file, done) || donePromise
+
+  return (process(content, file, done) || Promise.resolve()).then((content) => {
+    if (content) {
+      // async process correctly returned content
+      return content
+    }
+    // process called done() (Either old sync api or an async function that did not return content)
+    return donePromise
+  })
 }
 
 async function runProcessors (preprocessors, file, content) {
diff --git a/test/unit/preprocessor.spec.js b/test/unit/preprocessor.spec.js
@@ -80,6 +80,48 @@ describe('preprocessor', () => {
     })
   })
 
+  it('should get content if preprocessor is an async function or return Promise with content', (done) => {
+    const fakePreprocessor = sinon.spy(async (content, file, done) => {
+      file.path = file.path + '-preprocessed'
+      return 'new-content'
+    })
+
+    const injector = new di.Injector([{
+      'preprocessor:fake': ['factory', function () { return fakePreprocessor }]
+    }, emitterSetting])
+    pp = m.createPriorityPreprocessor({ '**/*.js': ['fake'] }, {}, null, injector)
+
+    const file = { originalPath: '/some/.dir/a.js', path: 'path' }
+
+    pp(file, () => {
+      expect(fakePreprocessor).to.have.been.called
+      expect(file.path).to.equal('path-preprocessed')
+      expect(file.content).to.equal('new-content')
+      done()
+    })
+  })
+
+  it('should get content if preprocessor is an async function still calling done()', (done) => {
+    const fakePreprocessor = sinon.spy(async (content, file, done) => {
+      file.path = file.path + '-preprocessed'
+      done(null, 'new-content')
+    })
+
+    const injector = new di.Injector([{
+      'preprocessor:fake': ['factory', function () { return fakePreprocessor }]
+    }, emitterSetting])
+    pp = m.createPriorityPreprocessor({ '**/*.js': ['fake'] }, {}, null, injector)
+
+    const file = { originalPath: '/some/.dir/a.js', path: 'path' }
+
+    pp(file, () => {
+      expect(fakePreprocessor).to.have.been.called
+      expect(file.path).to.equal('path-preprocessed')
+      expect(file.content).to.equal('new-content')
+      done()
+    })
+  })
+
   it('should check patterns after creation when invoked', (done) => {
     const fakePreprocessor = sinon.spy((content, file, done) => {
       file.path = file.path + '-preprocessed'

Original file line number	Diff line number	Diff line change
`@@ -24,7 +24,15 @@ function executeProcessor (process, file, content) {`
`24`	`24`	`}`
`25`	`25`	`}`
`26`	`26`	`})`
`27`		`- return process(content, file, done) \|\| donePromise`
	`27`	`+`
	`28`	`+ return (process(content, file, done) \|\| Promise.resolve()).then((content) => {`
	`29`	`+ if (content) {`
	`30`	`+ // async process correctly returned content`
	`31`	`+ return content`
	`32`	`+ }`
	`33`	`+ // process called done() (Either old sync api or an async function that did not return content)`
	`34`	`+ return donePromise`
	`35`	`+ })`
`28`	`36`	`}`
`29`	`37`
`30`	`38`	`async function runProcessors (preprocessors, file, content) {`