Skip to content

Commit

Permalink
feat(preprocessor): obey Pattern.isBinary when set (#3422)
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 authored Feb 20, 2020
1 parent 00d536f commit 708ae13
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 708ae13

Please sign in to comment.