Skip to content

Commit

Permalink
feat(preprocessor): obey Pattern.isBinary when set
Browse files Browse the repository at this point in the history
Add support for isBinary. If set, use it and don't probe file to detect binary status.
Fixes #3405
  • Loading branch information
johnjbarton committed Feb 10, 2020
1 parent 1fc51b0 commit 3c2a423
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 5 deletions.
3 changes: 2 additions & 1 deletion lib/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions lib/file-list.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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`)
Expand Down
5 changes: 4 additions & 1 deletion lib/file.js
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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 () {
Expand Down
6 changes: 5 additions & 1 deletion lib/preprocessor.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 })) {
Expand Down
38 changes: 38 additions & 0 deletions test/unit/preprocessor.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
}
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 3c2a423

Please sign in to comment.