diff --git a/.travis.yml b/.travis.yml index 48ec911e4..d6b753b57 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,15 +3,13 @@ node_js: - "8" - "10" - "node" -env: - - CXX=g++-4.8 addons: apt: sources: - ubuntu-toolchain-r-test packages: - - g++-4.8 -before_install: "sudo apt-get update && sudo apt-get install -y libcairo2-dev libjpeg8-dev libgif-dev optipng pngcrush pngquant libpango1.0-dev graphicsmagick libjpeg-turbo-progs inkscape gifsicle" + - graphicsmagick + - inkscape script: "npm run ci" after_success: " { - if (err) { - err.message = 'histogram: ' + err.message; return this.emit('error', err); } - if (data.colors.rgba < 256) { - // The image has fewer than 256 colors, run rawSrc through a PngQuant filter and proxy its events: - this.pngQuant = new PngQuant([ - data.colors.rgba < 2 ? 2 : data.colors.rgba - ]); - this.__defineGetter__('commandLine', () => this.pngQuant.commandLine); - for (const eventName of ['data', 'end', 'error']) { - this.pngQuant.on(eventName, () => { - // ... - this.emit.apply(this, eventName.concat(arguments)); - }); + + this.bufferedChunks = null; + + histogram(rawSrc, (err, data) => { + if (err) { + err.message = 'histogram: ' + err.message; + return this.emit('error', err); } - this.pngQuant.end(rawSrc); - } else { - // The image has too many colors. Emit all the buffered chunks at once when we aren't paused: - let hasEnded = false; - const emitRawSrcAndEnd = () => { - hasEnded = true; - this.emit('data', rawSrc); - this.emit('end'); - }; - if (this.isPaused) { - this.resume = function() { - setImmediate(function() { - if (!this.isPaused && !hasEnded) { - emitRawSrcAndEnd(); - } + if (data.colors.rgba < 256) { + // The image has fewer than 256 colors, run rawSrc through a PngQuant filter and proxy its events: + this.pngQuant = new PngQuant([ + data.colors.rgba < 2 ? 2 : data.colors.rgba + ]); + this.__defineGetter__('commandLine', () => this.pngQuant.commandLine); + for (const eventName of ['data', 'end', 'error']) { + this.pngQuant.on(eventName, (...args) => { + this.emit(eventName, ...args); }); - }; + } + this.pngQuant.end(rawSrc); } else { - emitRawSrcAndEnd(); + // The image has too many colors. Emit all the buffered chunks at once when we aren't paused: + let hasEnded = false; + const emitRawSrcAndEnd = () => { + hasEnded = true; + this.emit('data', rawSrc); + this.emit('end'); + }; + + if (this.isPaused) { + this.resume = function() { + setImmediate(function() { + if (!this.isPaused && !hasEnded) { + emitRawSrcAndEnd(); + } + }); + }; + } else { + emitRawSrcAndEnd(); + } } - } - }); -}; + }); + } -PngQuantWithHistogram.prototype.pause = function() { - this.isPaused = true; - if (this.pngQuant) { - this.pngQuant.pause(); + pause() { + this.isPaused = true; + if (this.pngQuant) { + this.pngQuant.pause(); + } } -}; -PngQuantWithHistogram.prototype.resume = function() { - this.isPaused = false; - if (this.pngQuant) { - this.pngQuant.resume(); + resume() { + this.isPaused = false; + if (this.pngQuant) { + this.pngQuant.resume(); + } } -}; +} module.exports = PngQuantWithHistogram; diff --git a/lib/transforms/processImages.js b/lib/transforms/processImages.js index 595685298..ace39e3c0 100644 --- a/lib/transforms/processImages.js +++ b/lib/transforms/processImages.js @@ -1,76 +1,35 @@ -const Promise = require('bluebird'); +const { promisify } = require('util'); +const promiseMap = require('p-map'); const childProcess = require('child_process'); const _ = require('lodash'); const urlTools = require('urltools'); const PngCrush = require('pngcrush'); const PngQuantWithHistogram = require('../PngQuantWithHistogram'); -const PngQuant = require('pngquant'); const OptiPng = require('optipng'); const JpegTran = require('jpegtran'); module.exports = (queryObj, options) => { options = options || {}; - const isAvailableByBinaryName = { - jpegtran: true, - pngcrush: true, - pngquant: true, - optipng: true, - gm: true - }; + let isGmAvailable = true; + return async function processImages(assetGraph) { const getFilterInfosAndTargetContentTypeFromQueryString = require('express-processimage/lib/getFilterInfosAndTargetContentTypeFromQueryString'); - await Promise.map( - Object.keys(isAvailableByBinaryName), - async binaryName => { - if ( - binaryName === 'jpegtran' || - binaryName === 'optipng' || - binaryName === 'pngcrush' || - binaryName === 'pngquant' - ) { - try { - await Promise.fromNode(cb => - ({ - jpegtran: JpegTran, - optipng: OptiPng, - pngcrush: PngCrush, - pngquant: PngQuant - }[binaryName].getBinaryPath(cb)) - ); - } catch (err) { - assetGraph.warn( - new Error( - 'processImages: ' + - binaryName + - ' not installed. Install it to get smaller ' + - (binaryName === 'jpegtran' ? 'jpgs' : 'pngs') - ) - ); - isAvailableByBinaryName[binaryName] = false; - } - } else { - try { - await Promise.fromNode(cb => childProcess.execFile(binaryName, cb)); - } catch (err) { - if (err.code === 127 || err.code === 'ENOENT') { - if (binaryName !== 'gm' && binaryName !== 'inkscape') { - assetGraph.warn( - new Error( - 'processImages: ' + - binaryName + - ' not installed. Install it to get smaller pngs' - ) - ); - } - isAvailableByBinaryName[binaryName] = false; - } - } - } + try { + await promisify(childProcess.execFile)('gm'); + } catch (err) { + isGmAvailable = false; + + if (err.code === 127 || err.code === 'ENOENT') { + assetGraph.warn( + new Error( + `processImages: Graphicsmagick not installed. Install it to get smaller pngs` + ) + ); } - ); + } - await Promise.map( + await promiseMap( assetGraph.findAssets( _.extend({ isImage: true, isInline: false }, queryObj) ), @@ -163,36 +122,31 @@ module.exports = (queryObj, options) => { if (targetContentType === 'image/png') { if ( (options.pngquant || autoLossless) && - isAvailableByBinaryName.pngquant && - PngQuantWithHistogram.histogramIsAvailable && operationNames.indexOf('pngquant') === -1 ) { filters.push(new PngQuantWithHistogram()); } if ( (options.optipng || autoLossless) && - isAvailableByBinaryName.optipng && operationNames.indexOf('optipng') === -1 ) { filters.push(new OptiPng()); } if ( (options.pngcrush || autoLossless) && - isAvailableByBinaryName.pngcrush && operationNames.indexOf('pngcrush') === -1 ) { - filters.push(new PngCrush(['-rem', 'alla'])); + filters.push(new PngCrush(['-rem', 'alla', '-noreduce'])); } } else if ( targetContentType === 'image/jpeg' && - isAvailableByBinaryName.jpegtran && (options.jpegtran || autoLossless) && operationNames.indexOf('jpegtran') === -1 ) { filters.push(new JpegTran(['-optimize'])); } - if (!isAvailableByBinaryName.gm) { + if (isGmAvailable) { for (let i = 0; i < filters.length; i += 1) { const filter = filters[i]; if (filter.operationName === 'gm') { @@ -221,6 +175,7 @@ module.exports = (queryObj, options) => { (leftOverQueryString ? '?' + leftOverQueryString : '') ); if (filters.length > 0) { + console.log('filters', filters.map(f => f.constructor.name)); await new Promise((resolve, reject) => { for (const [i, filter] of filters.entries()) { if (i < filters.length - 1) { @@ -243,7 +198,6 @@ module.exports = (queryObj, options) => { filterNameOrDescription + ': ' + err.message; - console.log(assetGraph.warn, assetGraph.error); if (hasNonAutoLosslessFilters) { assetGraph.warn(err); } diff --git a/package.json b/package.json index eadefe082..70aa07a82 100644 --- a/package.json +++ b/package.json @@ -41,27 +41,25 @@ ], "dependencies": { "assetgraph": "5.8.1", - "bluebird": "^3.5.0", + "assetgraph-sprite": "^3.0.1", "browserslist": "^4.4.2", "chalk": "^2.3.2", "esanimate": "^1.1.0", "estraverse": "^4.2.0", - "express-processimage": "8.1.0", + "express-processimage": "^8.1.0", "extend": "^3.0.0", + "histogram": "^3.0.1", "jpegtran": "^1.0.6", "lodash": "^4.14.1", "memoizesync": "^1.1.1", "optimist": "^0.6.1", "optipng": "^2.0.0", + "p-map": "^2.0.0", "passerror": "^1.1.1", "pngcrush": "^2.0.1", "pngquant": "^2.0.1", "urltools": "^0.4.1" }, - "optionalDependencies": { - "assetgraph-sprite": "^3.1.0", - "histogram": "^3.0.0" - }, "devDependencies": { "autoprefixer": "^9.0.0", "coveralls": "^3.0.0", diff --git a/test/bin/buildProduction.js b/test/bin/buildProduction.js index 34615aa96..cbeb0a97d 100644 --- a/test/bin/buildProduction.js +++ b/test/bin/buildProduction.js @@ -1,12 +1,10 @@ const expect = require('../unexpected-with-plugins'); const pathModule = require('path'); -const Promise = require('bluebird'); -const rimrafAsync = Promise.promisify(require('rimraf')); -const fs = Promise.promisifyAll(require('fs')); +const { promisify } = require('util'); +const rimraf = promisify(require('rimraf')); +const readFile = promisify(require('fs').readFile); const getTemporaryFilePath = require('gettemporaryfilepath'); -const childProcess = Promise.promisifyAll(require('child_process'), { - multiArgs: true -}); +const childProcess = require('child_process'); function run(commandAndArgs) { if (typeof commandAndArgs !== 'undefined' && !Array.isArray(commandAndArgs)) { @@ -26,9 +24,7 @@ function run(commandAndArgs) { expect.addAssertion( ' [when] run as a shell command ', function(expect, subject) { - return run(subject).then(function(stdout) { - return expect.shift(stdout); - }); + return run(subject).then(stdout => expect.shift(stdout)); } ); @@ -66,7 +62,7 @@ describe('buildProduction', function() { 'run as a shell command' ); - const builtIndexHtml = await fs.readFileAsync( + const builtIndexHtml = await readFile( pathModule.resolve(tmpDir, 'index.html'), 'utf-8' ); @@ -74,7 +70,7 @@ describe('buildProduction', function() { try { expect(builtIndexHtml, 'to contain', "foo['catch']=123"); } finally { - await rimrafAsync(tmpDir); + await rimraf(tmpDir); } }); @@ -103,7 +99,7 @@ describe('buildProduction', function() { 'run as a shell command' ); - const builtIndexHtml = await fs.readFileAsync( + const builtIndexHtml = await readFile( pathModule.resolve(tmpDir, 'index.html'), 'utf-8' ); @@ -111,7 +107,7 @@ describe('buildProduction', function() { try { expect(builtIndexHtml, 'to contain', 'foo.catch=123'); } finally { - await rimrafAsync(tmpDir); + await rimraf(tmpDir); process.chdir(originalDir); } }); @@ -147,7 +143,7 @@ describe('buildProduction', function() { 'run as a shell command' ); - const builtIndexHtml = await fs.readFileAsync( + const builtIndexHtml = await readFile( pathModule.resolve(tmpDir, 'index.html'), 'utf-8' ); @@ -155,7 +151,7 @@ describe('buildProduction', function() { try { expect(builtIndexHtml, 'to contain', "foo['catch']=123"); } finally { - await rimrafAsync(tmpDir); + await rimraf(tmpDir); } }); @@ -190,7 +186,7 @@ describe('buildProduction', function() { 'run as a shell command' ); - const builtIndexHtml = await fs.readFileAsync( + const builtIndexHtml = await readFile( pathModule.resolve(tmpDir, 'index.html'), 'utf-8' ); @@ -198,7 +194,7 @@ describe('buildProduction', function() { try { expect(builtIndexHtml, 'to contain', '::-ms-input-placeholder'); } finally { - await rimrafAsync(tmpDir); + await rimraf(tmpDir); } }); }); diff --git a/test/transforms/processImages.js b/test/transforms/processImages.js index 30c0bcefc..961dbbb0b 100644 --- a/test/transforms/processImages.js +++ b/test/transforms/processImages.js @@ -168,6 +168,190 @@ describe('processImages', function() { }); }); + describe('filters ordering', function() { + it('should handle filters default autolossless order', function() { + return new AssetGraph({ + root: __dirname + '/../../testdata/transforms/processImages/pngs/' + }) + .loadAssets({ + type: 'Css', + text: `.a { background-image: url(purplealpha24bit.png) }` + }) + .populate() + .processImages( + { type: 'Png' }, + { pngcrush: true, optipng: true, pngquant: true } + ) + .queue(function(assetGraph) { + var purpleAlpha24BitPngcrushed = assetGraph.findAssets({ + fileName: /^purplealpha24bit/ + })[0]; + + expect( + purpleAlpha24BitPngcrushed.rawSrc.length, + 'to be less than', + 8285 + ); + }); + }); + + it('should handle a single filter with autolossless: pngquant', function() { + return new AssetGraph({ + root: __dirname + '/../../testdata/transforms/processImages/pngs/' + }) + .loadAssets({ + type: 'Css', + text: `.a { background-image: url(purplealpha24bit.png?pngquant) }` + }) + .populate() + .processImages( + { type: 'Png' }, + { pngcrush: true, optipng: true, pngquant: true } + ) + .queue(function(assetGraph) { + var purpleAlpha24BitPngcrushed = assetGraph.findAssets({ + fileName: /^purplealpha24bit/ + })[0]; + + expect( + purpleAlpha24BitPngcrushed.rawSrc.length, + 'to be less than', + 8285 + ); + }); + }); + + it('should handle a single filter with autolossless: pngcrush', function() { + return new AssetGraph({ + root: __dirname + '/../../testdata/transforms/processImages/pngs/' + }) + .loadAssets({ + type: 'Css', + text: `.a { background-image: url(purplealpha24bit.png?pngcrush=-noreduce) }` + }) + .populate() + .processImages( + { type: 'Png' }, + { pngcrush: true, optipng: true, pngquant: true } + ) + .queue(function(assetGraph) { + var purpleAlpha24BitPngcrushed = assetGraph.findAssets({ + fileName: /^purplealpha24bit/ + })[0]; + + expect( + purpleAlpha24BitPngcrushed.rawSrc.length, + 'to be less than', + 8285 + ); + }); + }); + + it('should handle a single filter with autolossless: optipng', function() { + return new AssetGraph({ + root: __dirname + '/../../testdata/transforms/processImages/pngs/' + }) + .loadAssets({ + type: 'Css', + text: `.a { background-image: url(purplealpha24bit.png?optipng) }` + }) + .populate() + .processImages( + { type: 'Png' }, + { pngcrush: true, optipng: true, pngquant: true } + ) + .queue(function(assetGraph) { + var purpleAlpha24BitPngcrushed = assetGraph.findAssets({ + fileName: /^purplealpha24bit/ + })[0]; + + expect( + purpleAlpha24BitPngcrushed.rawSrc.length, + 'to be less than', + 8285 + ); + }); + }); + + it('should handle filters in order: pngquant, pngcrush, optipng', function() { + return new AssetGraph({ + root: __dirname + '/../../testdata/transforms/processImages/pngs/' + }) + .loadAssets({ + type: 'Css', + text: `.a { background-image: url(purplealpha24bit.png?pngquant&pngcrush=-noreduce&optipng) }` + }) + .populate() + .processImages( + { type: 'Png' }, + { pngcrush: true, optipng: true, pngquant: true } + ) + .queue(function(assetGraph) { + var purpleAlpha24BitPngcrushed = assetGraph.findAssets({ + fileName: /^purplealpha24bit/ + })[0]; + + expect( + purpleAlpha24BitPngcrushed.rawSrc.length, + 'to be less than', + 8285 + ); + }); + }); + + it('should handle filters in order: pngcrush, pngquant, optipng', function() { + return new AssetGraph({ + root: __dirname + '/../../testdata/transforms/processImages/pngs/' + }) + .loadAssets({ + type: 'Css', + text: `.a { background-image: url(purplealpha24bit.png?pngcrush=-noreduce&pngquant&optipng) }` + }) + .populate() + .processImages( + { type: 'Png' }, + { pngcrush: true, optipng: true, pngquant: true } + ) + .queue(function(assetGraph) { + var purpleAlpha24BitPngcrushed = assetGraph.findAssets({ + fileName: /^purplealpha24bit/ + })[0]; + + expect( + purpleAlpha24BitPngcrushed.rawSrc.length, + 'to be less than', + 8285 + ); + }); + }); + + it('should handle filters in order: optipng, pngquant, pngcrush', function() { + return new AssetGraph({ + root: __dirname + '/../../testdata/transforms/processImages/pngs/' + }) + .loadAssets({ + type: 'Css', + text: `.a { background-image: url(purplealpha24bit.png?optipng&pngquant&pngcrush=-noreduce) }` + }) + .populate() + .processImages( + { type: 'Png' }, + { pngcrush: true, optipng: true, pngquant: true } + ) + .queue(function(assetGraph) { + var purpleAlpha24BitPngcrushed = assetGraph.findAssets({ + fileName: /^purplealpha24bit/ + })[0]; + + expect( + purpleAlpha24BitPngcrushed.rawSrc.length, + 'to be less than', + 8285 + ); + }); + }); + }); + it('should handle a test case with a couple of pngs', function() { return new AssetGraph({ root: __dirname + '/../../testdata/transforms/processImages/pngs/'