feat(preprocessor): obey Pattern.isBinary when set (#3422)

johnjbarton · web-flow · commit 708ae133acc2 · 2020-02-20T09:42:26.000-08:00
Add support for isBinary. If set, use it and don't probe file to detect binary status. Fixes #3405
diff --git a/lib/config.js b/lib/config.js
@@ -32,14 +32,15 @@ try {
 } catch (e) {}
 
 class Pattern {
-  constructor (pattern, served, included, watched, nocache, type) {
+  constructor (pattern, served, included, watched, nocache, type, isBinary) {
     this.pattern = pattern
     this.served = helper.isDefined(served) ? served : true
     this.included = helper.isDefined(included) ? included : true
     this.watched = helper.isDefined(watched) ? watched : true
     this.nocache = helper.isDefined(nocache) ? nocache : false
     this.weight = helper.mmPatternWeight(pattern)
     this.type = type
+    this.isBinary = isBinary
   }
 
   compare (other) {
diff --git a/lib/file-list.js b/lib/file-list.js
@@ -61,7 +61,7 @@ class FileList {
 
     let lastCompletedRefresh = this._refreshing
     lastCompletedRefresh = Promise
-      .map(this._patterns, async ({ pattern, type, nocache }) => {
+      .map(this._patterns, async ({ pattern, type, nocache, isBinary }) => {
         if (helper.isUrlAbsolute(pattern)) {
           this.buckets.set(pattern, [new Url(pattern, type)])
           return
@@ -81,7 +81,7 @@ class FileList {
               return true
             }
           })
-          .map((path) => new File(path, mg.statCache[path].mtime, nocache, type))
+          .map((path) => new File(path, mg.statCache[path].mtime, nocache, type, isBinary))
 
         if (nocache) {
           log.debug(`Not preprocessing "${pattern}" due to nocache`)
diff --git a/lib/file.js b/lib/file.js
@@ -4,7 +4,7 @@
  * File object used for tracking files in `file-list.js`.
  */
 class File {
-  constructor (path, mtime, doNotCache, type) {
+  constructor (path, mtime, doNotCache, type, isBinary) {
     // used for serving (processed path, eg some/file.coffee -> some/file.coffee.js)
     this.path = path
 
@@ -24,6 +24,9 @@ class File {
     this.doNotCache = doNotCache === undefined ? false : doNotCache
 
     this.type = type
+
+    // Tri state: null means probe file for binary.
+    this.isBinary = isBinary === undefined ? null : isBinary
   }
 
   toString () {
diff --git a/lib/preprocessor.js b/lib/preprocessor.js
@@ -102,7 +102,11 @@ function createPriorityPreprocessor (config = {}, preprocessorPriority, basePath
 
   return async function preprocess (file) {
     const buffer = await tryToRead(file.originalPath, log)
-    const isBinary = await isBinaryFile(buffer, buffer.length)
+    let isBinary = file.isBinary
+    if (isBinary == null) {
+      // Pattern did not specify, probe for it.
+      isBinary = await isBinaryFile(buffer, buffer.length)
+    }
 
     const preprocessorNames = Object.keys(config).reduce((ppNames, pattern) => {
       if (mm(file.originalPath, pattern, { dot: true })) {
diff --git a/test/unit/preprocessor.spec.js b/test/unit/preprocessor.spec.js
@@ -21,6 +21,7 @@ describe('preprocessor', () => {
         'a.txt': mocks.fs.file(0, 'some-text'),
         'photo.png': mocks.fs.file(0, binarydata),
         'CAM_PHOTO.JPG': mocks.fs.file(0, binarydata),
+        'proto.pb': mocks.fs.file(0, Buffer.from('mixed-content', 'utf8')),
         '.dir': {
           'a.js': mocks.fs.file(0, 'content')
         }
@@ -325,6 +326,43 @@ describe('preprocessor', () => {
     expect(file.content).to.be.an.instanceof(Buffer)
   })
 
+  it('should not preprocess files configured to be binary', async () => {
+    const fakePreprocessor = sinon.spy((content, file, done) => {
+      done(null, content)
+    })
+
+    const injector = new di.Injector([{
+      'preprocessor:fake': ['factory', function () { return fakePreprocessor }]
+    }, emitterSetting])
+
+    const pp = m.createPriorityPreprocessor({ '**/*': ['fake'] }, {}, null, injector)
+
+    const file = { originalPath: '/some/proto.pb', path: 'path', isBinary: true }
+
+    await pp(file)
+    expect(fakePreprocessor).not.to.have.been.called
+    expect(file.content).to.be.an.instanceof(Buffer)
+  })
+
+  it('should preprocess files configured not to be binary', async () => {
+    const fakePreprocessor = sinon.spy((content, file, done) => {
+      done(null, content)
+    })
+
+    const injector = new di.Injector([{
+      'preprocessor:fake': ['factory', function () { return fakePreprocessor }]
+    }, emitterSetting])
+
+    const pp = m.createPriorityPreprocessor({ '**/*': ['fake'] }, {}, null, injector)
+
+    // Explicit false for isBinary
+    const file = { originalPath: '/some/proto.pb', path: 'path', isBinary: false }
+
+    await pp(file)
+    expect(fakePreprocessor).to.have.been.calledOnce
+    expect(typeof file.content).to.equal('string')
+  })
+
   it('should preprocess binary files if handleBinaryFiles=true', async () => {
     const fakePreprocessor = sinon.spy((content, file, done) => {
       done(null, content)
