Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Feb 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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