From 9851cbf71408dbb60c03acb64f46f3ab0fcb9692 Mon Sep 17 00:00:00 2001 From: Konstantin Mamaev Date: Tue, 2 Apr 2019 19:10:01 +0300 Subject: [PATCH 1/4] WIP: Promise for everything * Replace all sync operations to async --- src/index.js | 5 ++- src/lib/decl-processor.js | 53 +++++++++++++++------- src/lib/get-file.js | 67 +++++++++++++++++++++------- src/type/copy.js | 93 +++++++++++++++++++++++++++------------ src/type/custom.js | 2 +- src/type/inline.js | 80 +++++++++++++++++++-------------- src/type/rebase.js | 4 +- 7 files changed, 205 insertions(+), 99 deletions(-) diff --git a/src/index.js b/src/index.js index 9d09dd0..5eeaf1c 100644 --- a/src/index.js +++ b/src/index.js @@ -13,13 +13,16 @@ module.exports = postcss.plugin('postcss-url', (options) => { options = options || {}; return function(styles, result) { + const promises = []; const opts = result.opts; const from = opts.from ? path.dirname(opts.from) : '.'; const to = opts.to ? path.dirname(opts.to) : from; styles.walkDecls((decl) => - declProcessor(from, to, options, result, decl) + promises.push(declProcessor(from, to, options, result, decl)) ); + + return Promise.all(promises); }; }); diff --git a/src/lib/decl-processor.js b/src/lib/decl-processor.js index 0a7cbc0..80dce9b 100644 --- a/src/lib/decl-processor.js +++ b/src/lib/decl-processor.js @@ -64,7 +64,7 @@ const wrapUrlProcessor = (urlProcessor, result, decl) => { }); return (asset, dir, option) => - urlProcessor(asset, dir, option, decl, warn, result, addDependency); + urlProcessor(asset, dir, option, decl, warn, addDependency); }; /** @@ -80,14 +80,14 @@ const getPattern = (decl) => * @param {Options} options * @param {Result} result * @param {Decl} decl - * @returns {String|undefined} + * @returns {Promise} */ const replaceUrl = (url, dir, options, result, decl) => { const asset = prepareAsset(url, dir, decl); const matchedOptions = matchOptions(asset, options); - if (!matchedOptions) return; + if (!matchedOptions) return Promise.resolve(); const process = (option) => { const wrappedUrlProcessor = wrapUrlProcessor(getUrlProcessor(option.url), result, decl); @@ -95,13 +95,21 @@ const replaceUrl = (url, dir, options, result, decl) => { return wrappedUrlProcessor(asset, dir, option); }; + const resultPromise; if (Array.isArray(matchedOptions)) { - matchedOptions.forEach((option) => asset.url = process(option)); + resultPromise = Promise.resolve(); + matchedOptions.forEach((option) => { + resultPromise = resultPromise.then(() => { + return process(option); + }); + }); } else { - asset.url = process(matchedOptions); + resultPromise = process(matchedOptions); } - return asset.url; + return resultPromise.then((url) => { + asset.url = url; + }); }; /** @@ -110,31 +118,44 @@ const replaceUrl = (url, dir, options, result, decl) => { * @param {PostcssUrl~Options} options * @param {Result} result * @param {Decl} decl - * @returns {PostcssUrl~DeclProcessor} + * @returns {Promise} */ const declProcessor = (from, to, options, result, decl) => { const dir = { from, to, file: getDirDeclFile(decl) }; const pattern = getPattern(decl); - if (!pattern) return; + if (!pattern) return Promise.resolve(); + + let id = 0; + const promises = []; decl.value = decl.value - .replace(pattern, (matched, before, url, after) => { - const newUrl = replaceUrl(url, dir, options, result, decl); + .replace(pattern, (matched, before, url, after) => { + const newUrlPromise = replaceUrl(url, dir, options, result, decl);; + + const marker = `::id${id++}`; - if (!newUrl) return matched; + promises.push( + newUrlPromise + .then((newUrl) => { + if (!newUrl) return matched; - if (WITH_QUOTES.test(newUrl) && WITH_QUOTES.test(after)) { + if (WITH_QUOTES.test(newUrl) && WITH_QUOTES.test(after)) { before = before.slice(0, -1); after = after.slice(1); - } + } - return `${before}${newUrl}${after}`; - }); + decl.value = decl.value.replace(marker, `${before}${newUrl}${after}`); + }) + ); + + return marker; + }); + + return Promise.all(promises); }; module.exports = { - replaceUrl, declProcessor }; diff --git a/src/lib/get-file.js b/src/lib/get-file.js index 7bb4b02..bcadaa7 100644 --- a/src/lib/get-file.js +++ b/src/lib/get-file.js @@ -5,31 +5,64 @@ const mime = require('mime'); const getPathByBasePath = require('./paths').getPathByBasePath; +const readFileAsync = (filePath) => { + return new Promise((resolse, reject) => { + fs.readFile(filePath, (err, data) => { + if (err) { + reject(err); + } + resolve(data); + }); + }); +}; + +const existFileAsync = (filePath) => { + new Promise((resolve, reject) => + fs.access(filePath, (err) => { + if (err) { + reject(); + } + resolve(path); + }) + ) +}; + +const oneSuccess = (promises) => { + return Promise.all(promises.map(p => { + return p.then( + val => Promise.reject(val), + err => Promise.resolve(err) + ); + })).then( + errors => Promise.reject(errors), + val => Promise.resolve(val) + ); +} + /** * * @param {PostcssUrl~Asset} asset * @param {PostcssUrl~Options} options * @param {PostcssUrl~Dir} dir * @param {Function} warn - * @returns {PostcssUrl~File} + * @returns {Promise} */ const getFile = (asset, options, dir, warn) => { - const paths = options.basePath - ? getPathByBasePath(options.basePath, dir.from, asset.pathname) - : [asset.absolutePath]; - const filePath = paths.find(fs.existsSync); - - if (!filePath) { - warn(`Can't read file '${paths.join()}', ignoring`); - - return; - } - - return { - path: filePath, - contents: fs.readFileSync(filePath), - mimeType: mime.getType(filePath) - }; + const paths = options.basePath ? + getPathByBasePath(options.basePath, dir.from, asset.pathname) : + [asset.absolutePath]; + + return oneSuccess(paths.map(path => existFileAsync(path))) + .then(path => readFileAsync(path)) + .then(contents => ({ + path: filePath, + contents: contents, + mimeType: mime.getType(filePath) + })) + .catch(() => { + warn(`Can't read file '${paths.join()}', ignoring`); + return; + }); }; module.exports = getFile; diff --git a/src/type/copy.js b/src/type/copy.js index 2f1da06..dd8a1d4 100644 --- a/src/type/copy.js +++ b/src/type/copy.js @@ -13,9 +13,46 @@ const getAssetsPath = paths.getAssetsPath; const normalize = paths.normalize; const getHashName = (file, options) => - (options && options.append ? (`${path.basename(file.path, path.extname(file.path))}_`) : '') - + calcHash(file.contents, options) - + path.extname(file.path); + (options && options.append ? (`${path.basename(file.path, path.extname(file.path))}_`) : '') + + calcHash(file.contents, options) + + path.extname(file.path); + +const createDirAsync = (path) => { + return Promise((resolve, reject) => { + mkdirp(path, (err) => { + if (err) reject(err); + resolve(); + }) + }) +}; + +const writeFileAsync = (file, src) => { + return new Promise((resolve, reject) => { + fs.open(dest, 'wx', (err, fd) => { + if (err) { + if (err.code === 'EEXIST') { + resolve(); + } + + reject(err); + } + + resolve(fd); + }) + }) + .then(fd => { + if (!fd) return; + + return new Promise((resolve, reject) => { + fs.writeFile(newAssetPath, file.contents, (err) => { + if (err) { + reject(err); + } + resolve(); + }); + }) + }); +}; /** * Copy images from readed from url() to an specific assets destination @@ -33,37 +70,35 @@ const getHashName = (file, options) => * @param {Result} result * @param {Function} addDependency * - * @returns {String|Undefined} + * @returns {Promise} */ -module.exports = function processCopy(asset, dir, options, decl, warn, result, addDependency) { - if (!options.assetsPath && dir.from === dir.to) { - warn('Option `to` of postcss is required, ignoring'); - - return; - } - - const file = getFile(asset, options, dir, warn); - - if (!file) return; - const assetRelativePath = options.useHash - ? getHashName(file, options.hashOptions) - : asset.relativePath; +module.exports = function processCopy(asset, dir, options, decl, warn, addDependency) { + if (!options.assetsPath && dir.from === dir.to) { + warn('Option `to` of postcss is required, ignoring'); - const targetDir = getTargetDir(dir); - const newAssetBaseDir = getAssetsPath(targetDir, options.assetsPath); - const newAssetPath = path.join(newAssetBaseDir, assetRelativePath); - const newRelativeAssetPath = normalize( - path.relative(targetDir, newAssetPath) - ); + return Promise.resolve(); + } - mkdirp.sync(path.dirname(newAssetPath)); + const newRelativeAssetPath = generateNewPath(file, dir, options); - if (!fs.existsSync(newAssetPath)) { - fs.writeFileSync(newAssetPath, file.contents); - } + return getFile(asset, options, dir, warn) + .then(file => { + const assetRelativePath = options.useHash ? + getHashName(file, options.hashOptions) : + asset.relativePath; - addDependency(file.path); + const targetDir = getTargetDir(dir); + const newAssetBaseDir = getAssetsPath(targetDir, options.assetsPath); + const newAssetPath = path.join(newAssetBaseDir, assetRelativePath); + const newRelativeAssetPath = normalize(path.relative(targetDir, newAssetPath)); - return `${newRelativeAssetPath}${asset.search}${asset.hash}`; + return createDirAsync(path.dirname(newAssetPath)) + .then(() => writeFileAsync(file, newAssetPath)) + .then(() => { + addDependency(file.path); + return `${newRelativeAssetPath}${asset.search}${asset.hash}`; + }); + } + ); }; diff --git a/src/type/custom.js b/src/type/custom.js index 4c610d4..68338dd 100644 --- a/src/type/custom.js +++ b/src/type/custom.js @@ -6,7 +6,7 @@ * @param {PostcssUrl~Dir} dir * @param {PostcssUrl~Option} options * - * @returns {String|Undefined} + * @returns {Promise} */ module.exports = function getCustomProcessor(asset, dir, options) { return options.url.apply(null, arguments); diff --git a/src/type/inline.js b/src/type/inline.js index d25c29c..c7f405d 100644 --- a/src/type/inline.js +++ b/src/type/inline.js @@ -15,7 +15,7 @@ const getFile = require('../lib/get-file'); * * @returns {String|Undefined} */ -function processFallback(originUrl, dir, options) { +const processFallback = (originUrl, dir, options) => { if (typeof options.fallback === 'function') { return options.fallback.apply(null, arguments); } @@ -25,10 +25,34 @@ function processFallback(originUrl, dir, options) { case 'rebase': return processRebase.apply(null, arguments); default: - return; + return Promise.resolve(); } } +const inlineProcess = (file, asset, warn, options) => { + const isSvg = file.mimeType === 'image/svg+xml'; + const defaultEncodeType = isSvg ? 'encodeURIComponent' : 'base64'; + const encodeType = options.encodeType || defaultEncodeType; + + // Warn for svg with hashes/fragments + if (isSvg && asset.hash && !options.ignoreFragmentWarning) { + // eslint-disable-next-line max-len + warn(`Image type is svg and link contains #. Postcss-url cant handle svg fragments. SVG file fully inlined. ${file.path}`); + } + + addDependency(file.path); + + const optimizeSvgEncode = isSvg && options.optimizeSvgEncode; + const encodedStr = encodeFile(file, encodeType, optimizeSvgEncode); + const resultValue = options.includeUriFragment && asset.hash + ? encodedStr + asset.hash + : encodedStr; + + // wrap url by quotes if percent-encoded svg + return isSvg && encodeType !== 'base64' ? `"${resultValue}"` : resultValue; +}; + + /** * Inline image in url() * @@ -41,48 +65,38 @@ function processFallback(originUrl, dir, options) { * @param {Result} result * @param {Function} addDependency * - * @returns {String|Undefined} + * @returns {Promise} */ // eslint-disable-next-line complexity -module.exports = function(asset, dir, options, decl, warn, result, addDependency) { - const file = getFile(asset, options, dir, warn); - - if (!file) return; +module.exports = function(asset, dir, options, decl, warn, addDependency) { + return getFile(asset, options, dir, warn) + .then(file => { + if (!file) return; - if (!file.mimeType) { - warn(`Unable to find asset mime-type for ${file.path}`); + if (!file.mimeType) { + warn(`Unable to find asset mime-type for ${file.path}`); - return; - } + return; + } - const maxSize = (options.maxSize || 0) * 1024; + const maxSize = (options.maxSize || 0) * 1024; - if (maxSize) { - const stats = fs.statSync(file.path); + if (maxSize) { + const size = Buffer.byteLength(file.contents); - if (stats.size >= maxSize) { - return processFallback.apply(this, arguments); + if (stats.size >= maxSize) { + return processFallback.apply(this, arguments); + } } - } - const isSvg = file.mimeType === 'image/svg+xml'; - const defaultEncodeType = isSvg ? 'encodeURIComponent' : 'base64'; - const encodeType = options.encodeType || defaultEncodeType; + return inlineProcess(file, asset, warn, options); + }); + + + + - // Warn for svg with hashes/fragments - if (isSvg && asset.hash && !options.ignoreFragmentWarning) { - // eslint-disable-next-line max-len - warn(`Image type is svg and link contains #. Postcss-url cant handle svg fragments. SVG file fully inlined. ${file.path}`); - } - addDependency(file.path); - const optimizeSvgEncode = isSvg && options.optimizeSvgEncode; - const encodedStr = encodeFile(file, encodeType, optimizeSvgEncode); - const resultValue = options.includeUriFragment && asset.hash - ? encodedStr + asset.hash - : encodedStr; - // wrap url by quotes if percent-encoded svg - return isSvg && encodeType !== 'base64' ? `"${resultValue}"` : resultValue; }; diff --git a/src/type/rebase.js b/src/type/rebase.js index d327618..658c2bf 100644 --- a/src/type/rebase.js +++ b/src/type/rebase.js @@ -13,7 +13,7 @@ const getAssetsPath = paths.getAssetsPath; * @param {PostcssUrl~Dir} dir * @param {PostcssUrl~Option} options * - * @returns {String|Undefined} + * @returns {Promise} */ module.exports = function(asset, dir, options) { const dest = getAssetsPath(dir.to, options && options.assetsPath || ''); @@ -21,5 +21,5 @@ module.exports = function(asset, dir, options) { path.relative(dest, asset.absolutePath) ); - return `${rebasedUrl}${asset.search}${asset.hash}`; + return Promise.resolve(`${rebasedUrl}${asset.search}${asset.hash}`); }; From 85c742ea5011b05084225d4ce5046623287a029c Mon Sep 17 00:00:00 2001 From: Konstantin Mamaev Date: Tue, 2 Apr 2019 19:24:23 +0300 Subject: [PATCH 2/4] Fix lint issues --- src/index.js | 4 +- src/lib/decl-processor.js | 48 ++++++++-------- src/lib/get-file.js | 77 ++++++++++++------------- src/type/copy.js | 117 +++++++++++++++++++------------------- src/type/inline.js | 77 +++++++++++-------------- 5 files changed, 156 insertions(+), 167 deletions(-) diff --git a/src/index.js b/src/index.js index 5eeaf1c..402705e 100644 --- a/src/index.js +++ b/src/index.js @@ -19,10 +19,10 @@ module.exports = postcss.plugin('postcss-url', (options) => { const to = opts.to ? path.dirname(opts.to) : from; styles.walkDecls((decl) => - promises.push(declProcessor(from, to, options, result, decl)) + promises.push(declProcessor(from, to, options, result, decl)) ); - return Promise.all(promises); + return Promise.all(promises); }; }); diff --git a/src/lib/decl-processor.js b/src/lib/decl-processor.js index 80dce9b..11e23a8 100644 --- a/src/lib/decl-processor.js +++ b/src/lib/decl-processor.js @@ -95,20 +95,21 @@ const replaceUrl = (url, dir, options, result, decl) => { return wrappedUrlProcessor(asset, dir, option); }; - const resultPromise; + let resultPromise; + if (Array.isArray(matchedOptions)) { resultPromise = Promise.resolve(); matchedOptions.forEach((option) => { - resultPromise = resultPromise.then(() => { - return process(option); - }); + resultPromise = resultPromise.then(() => { + return process(option); + }); }); } else { - resultPromise = process(matchedOptions); + resultPromise = process(matchedOptions); } - return resultPromise.then((url) => { - asset.url = url; + return resultPromise.then((newUrl) => { + asset.url = newUrl; }); }; @@ -130,27 +131,26 @@ const declProcessor = (from, to, options, result, decl) => { const promises = []; decl.value = decl.value - .replace(pattern, (matched, before, url, after) => { - const newUrlPromise = replaceUrl(url, dir, options, result, decl);; - - const marker = `::id${id++}`; + .replace(pattern, (matched, before, url, after) => { + const newUrlPromise = replaceUrl(url, dir, options, result, decl); + const marker = `::id${id++}`; - promises.push( - newUrlPromise - .then((newUrl) => { - if (!newUrl) return matched; + promises.push( + newUrlPromise + .then((newUrl) => { + if (!newUrl) return matched; - if (WITH_QUOTES.test(newUrl) && WITH_QUOTES.test(after)) { - before = before.slice(0, -1); - after = after.slice(1); - } + if (WITH_QUOTES.test(newUrl) && WITH_QUOTES.test(after)) { + before = before.slice(0, -1); + after = after.slice(1); + } - decl.value = decl.value.replace(marker, `${before}${newUrl}${after}`); - }) - ); + decl.value = decl.value.replace(marker, `${before}${newUrl}${after}`); + }) + ); - return marker; - }); + return marker; + }); return Promise.all(promises); }; diff --git a/src/lib/get-file.js b/src/lib/get-file.js index bcadaa7..c9d718f 100644 --- a/src/lib/get-file.js +++ b/src/lib/get-file.js @@ -6,38 +6,38 @@ const mime = require('mime'); const getPathByBasePath = require('./paths').getPathByBasePath; const readFileAsync = (filePath) => { - return new Promise((resolse, reject) => { - fs.readFile(filePath, (err, data) => { - if (err) { - reject(err); - } - resolve(data); + return new Promise((resolve, reject) => { + fs.readFile(filePath, (err, data) => { + if (err) { + reject(err); + } + resolve(data); + }); }); - }); }; const existFileAsync = (filePath) => { - new Promise((resolve, reject) => - fs.access(filePath, (err) => { - if (err) { - reject(); - } - resolve(path); - }) - ) + return new Promise((resolve, reject) => + fs.access(filePath, (err) => { + if (err) { + reject(); + } + resolve(filePath); + }) + ); }; const oneSuccess = (promises) => { - return Promise.all(promises.map(p => { - return p.then( - val => Promise.reject(val), - err => Promise.resolve(err) + return Promise.all(promises.map((p) => { + return p.then( + (val) => Promise.reject(val), + (err) => Promise.resolve(err) + ); + })).then( + (errors) => Promise.reject(errors), + (val) => Promise.resolve(val) ); - })).then( - errors => Promise.reject(errors), - val => Promise.resolve(val) - ); -} +}; /** * @@ -48,21 +48,22 @@ const oneSuccess = (promises) => { * @returns {Promise} */ const getFile = (asset, options, dir, warn) => { - const paths = options.basePath ? - getPathByBasePath(options.basePath, dir.from, asset.pathname) : - [asset.absolutePath]; + const paths = options.basePath + ? getPathByBasePath(options.basePath, dir.from, asset.pathname) + : [asset.absolutePath]; - return oneSuccess(paths.map(path => existFileAsync(path))) - .then(path => readFileAsync(path)) - .then(contents => ({ - path: filePath, - contents: contents, - mimeType: mime.getType(filePath) - })) - .catch(() => { - warn(`Can't read file '${paths.join()}', ignoring`); - return; - }); + return oneSuccess(paths.map((path) => existFileAsync(path))) + .then((path) => readFileAsync(path) + .then((contents) => ({ + path, + contents, + mimeType: mime.getType(path) + }))) + .catch(() => { + warn(`Can't read file '${paths.join()}', ignoring`); + + return; + }); }; module.exports = getFile; diff --git a/src/type/copy.js b/src/type/copy.js index dd8a1d4..0f8fa6a 100644 --- a/src/type/copy.js +++ b/src/type/copy.js @@ -13,45 +13,45 @@ const getAssetsPath = paths.getAssetsPath; const normalize = paths.normalize; const getHashName = (file, options) => - (options && options.append ? (`${path.basename(file.path, path.extname(file.path))}_`) : '') + - calcHash(file.contents, options) + - path.extname(file.path); - -const createDirAsync = (path) => { - return Promise((resolve, reject) => { - mkdirp(path, (err) => { - if (err) reject(err); - resolve(); - }) - }) + (options && options.append ? (`${path.basename(file.path, path.extname(file.path))}_`) : '') + + calcHash(file.contents, options) + + path.extname(file.path); + +const createDirAsync = (dirPath) => { + return new Promise((resolve, reject) => { + mkdirp(dirPath, (err) => { + if (err) reject(err); + resolve(); + }); + }); }; -const writeFileAsync = (file, src) => { - return new Promise((resolve, reject) => { - fs.open(dest, 'wx', (err, fd) => { - if (err) { - if (err.code === 'EEXIST') { - resolve(); - } +const writeFileAsync = (file, dest) => { + return new Promise((resolve, reject) => { + fs.open(dest, 'wx', (err, fd) => { + if (err) { + if (err.code === 'EEXIST') { + resolve(); + } - reject(err); - } + reject(err); + } - resolve(fd); - }) + resolve(fd); + }); }) - .then(fd => { - if (!fd) return; - - return new Promise((resolve, reject) => { - fs.writeFile(newAssetPath, file.contents, (err) => { - if (err) { - reject(err); - } - resolve(); + .then((fd) => { + if (!fd) return; + + return new Promise((resolve, reject) => { + fs.writeFile(dest, file.contents, (err) => { + if (err) { + reject(err); + } + resolve(); + }); + }); }); - }) - }); }; /** @@ -74,31 +74,30 @@ const writeFileAsync = (file, src) => { */ module.exports = function processCopy(asset, dir, options, decl, warn, addDependency) { - if (!options.assetsPath && dir.from === dir.to) { - warn('Option `to` of postcss is required, ignoring'); - - return Promise.resolve(); - } - - const newRelativeAssetPath = generateNewPath(file, dir, options); - - return getFile(asset, options, dir, warn) - .then(file => { - const assetRelativePath = options.useHash ? - getHashName(file, options.hashOptions) : - asset.relativePath; - - const targetDir = getTargetDir(dir); - const newAssetBaseDir = getAssetsPath(targetDir, options.assetsPath); - const newAssetPath = path.join(newAssetBaseDir, assetRelativePath); - const newRelativeAssetPath = normalize(path.relative(targetDir, newAssetPath)); - - return createDirAsync(path.dirname(newAssetPath)) - .then(() => writeFileAsync(file, newAssetPath)) - .then(() => { - addDependency(file.path); - return `${newRelativeAssetPath}${asset.search}${asset.hash}`; - }); + if (!options.assetsPath && dir.from === dir.to) { + warn('Option `to` of postcss is required, ignoring'); + + return Promise.resolve(); + } + + return getFile(asset, options, dir, warn) + .then((file) => { + const assetRelativePath = options.useHash + ? getHashName(file, options.hashOptions) + : asset.relativePath; + + const targetDir = getTargetDir(dir); + const newAssetBaseDir = getAssetsPath(targetDir, options.assetsPath); + const newAssetPath = path.join(newAssetBaseDir, assetRelativePath); + const newRelativeAssetPath = normalize(path.relative(targetDir, newAssetPath)); + + return createDirAsync(path.dirname(newAssetPath)) + .then(() => writeFileAsync(file, newAssetPath)) + .then(() => { + addDependency(file.path); + + return `${newRelativeAssetPath}${asset.search}${asset.hash}`; + }); } - ); + ); }; diff --git a/src/type/inline.js b/src/type/inline.js index c7f405d..f100e07 100644 --- a/src/type/inline.js +++ b/src/type/inline.js @@ -1,7 +1,5 @@ 'use strict'; -const fs = require('fs'); - const processCopy = require('./copy'); const processRebase = require('./rebase'); @@ -27,32 +25,31 @@ const processFallback = (originUrl, dir, options) => { default: return Promise.resolve(); } -} +}; -const inlineProcess = (file, asset, warn, options) => { - const isSvg = file.mimeType === 'image/svg+xml'; - const defaultEncodeType = isSvg ? 'encodeURIComponent' : 'base64'; - const encodeType = options.encodeType || defaultEncodeType; +const inlineProcess = (file, asset, warn, addDependency, options) => { + const isSvg = file.mimeType === 'image/svg+xml'; + const defaultEncodeType = isSvg ? 'encodeURIComponent' : 'base64'; + const encodeType = options.encodeType || defaultEncodeType; - // Warn for svg with hashes/fragments - if (isSvg && asset.hash && !options.ignoreFragmentWarning) { - // eslint-disable-next-line max-len - warn(`Image type is svg and link contains #. Postcss-url cant handle svg fragments. SVG file fully inlined. ${file.path}`); - } + // Warn for svg with hashes/fragments + if (isSvg && asset.hash && !options.ignoreFragmentWarning) { + // eslint-disable-next-line max-len + warn(`Image type is svg and link contains #. Postcss-url cant handle svg fragments. SVG file fully inlined. ${file.path}`); + } - addDependency(file.path); + addDependency(file.path); - const optimizeSvgEncode = isSvg && options.optimizeSvgEncode; - const encodedStr = encodeFile(file, encodeType, optimizeSvgEncode); - const resultValue = options.includeUriFragment && asset.hash - ? encodedStr + asset.hash - : encodedStr; + const optimizeSvgEncode = isSvg && options.optimizeSvgEncode; + const encodedStr = encodeFile(file, encodeType, optimizeSvgEncode); + const resultValue = options.includeUriFragment && asset.hash + ? encodedStr + asset.hash + : encodedStr; - // wrap url by quotes if percent-encoded svg - return isSvg && encodeType !== 'base64' ? `"${resultValue}"` : resultValue; + // wrap url by quotes if percent-encoded svg + return isSvg && encodeType !== 'base64' ? `"${resultValue}"` : resultValue; }; - /** * Inline image in url() * @@ -70,33 +67,25 @@ const inlineProcess = (file, asset, warn, options) => { // eslint-disable-next-line complexity module.exports = function(asset, dir, options, decl, warn, addDependency) { return getFile(asset, options, dir, warn) - .then(file => { - if (!file) return; - - if (!file.mimeType) { - warn(`Unable to find asset mime-type for ${file.path}`); - - return; - } - - const maxSize = (options.maxSize || 0) * 1024; - - if (maxSize) { - const size = Buffer.byteLength(file.contents); - - if (stats.size >= maxSize) { - return processFallback.apply(this, arguments); - } - } - - return inlineProcess(file, asset, warn, options); - }); - - + .then((file) => { + if (!file) return; + if (!file.mimeType) { + warn(`Unable to find asset mime-type for ${file.path}`); + return; + } + const maxSize = (options.maxSize || 0) * 1024; + if (maxSize) { + const size = Buffer.byteLength(file.contents); + if (size >= maxSize) { + return processFallback.apply(this, arguments); + } + } + return inlineProcess(file, asset, warn, addDependency, options); + }); }; From b1efa54da5e0cd8e994770ebef9f542f8123127e Mon Sep 17 00:00:00 2001 From: Konstantin Mamaev Date: Wed, 3 Apr 2019 21:28:28 +0300 Subject: [PATCH 3/4] Fresh tests --- src/lib/decl-processor.js | 27 +++++++------ src/type/copy.js | 9 ++++- src/type/custom.js | 2 +- src/type/inline.js | 6 +-- src/type/rebase.js | 2 +- test/lib/get-file.js | 55 ++++++++++++++----------- test/misc/messages.js | 18 +++++++-- test/setup.js | 2 +- test/type/copy.js | 49 ++++++++++++---------- test/type/inline.js | 85 ++++++++++++++++++++------------------- test/type/rebase.js | 21 +++++----- 11 files changed, 156 insertions(+), 120 deletions(-) diff --git a/src/lib/decl-processor.js b/src/lib/decl-processor.js index 11e23a8..6c40b32 100644 --- a/src/lib/decl-processor.js +++ b/src/lib/decl-processor.js @@ -64,7 +64,7 @@ const wrapUrlProcessor = (urlProcessor, result, decl) => { }); return (asset, dir, option) => - urlProcessor(asset, dir, option, decl, warn, addDependency); + urlProcessor(asset, dir, option, decl, warn, result, addDependency); }; /** @@ -95,21 +95,26 @@ const replaceUrl = (url, dir, options, result, decl) => { return wrappedUrlProcessor(asset, dir, option); }; - let resultPromise; + let resultPromise = Promise.resolve(); if (Array.isArray(matchedOptions)) { - resultPromise = Promise.resolve(); - matchedOptions.forEach((option) => { - resultPromise = resultPromise.then(() => { - return process(option); - }); - }); + for (let i = 0; i < matchedOptions.length; i++) { + resultPromise = resultPromise + .then(() => process(matchedOptions[i])) + .then((newUrl) => { + asset.url = newUrl; + + return newUrl; + }); + } } else { resultPromise = process(matchedOptions); } return resultPromise.then((newUrl) => { asset.url = newUrl; + + return newUrl; }); }; @@ -127,13 +132,11 @@ const declProcessor = (from, to, options, result, decl) => { if (!pattern) return Promise.resolve(); - let id = 0; const promises = []; decl.value = decl.value .replace(pattern, (matched, before, url, after) => { const newUrlPromise = replaceUrl(url, dir, options, result, decl); - const marker = `::id${id++}`; promises.push( newUrlPromise @@ -145,11 +148,11 @@ const declProcessor = (from, to, options, result, decl) => { after = after.slice(1); } - decl.value = decl.value.replace(marker, `${before}${newUrl}${after}`); + decl.value = decl.value.replace(matched, `${before}${newUrl}${after}`); }) ); - return marker; + return matched; }); return Promise.all(promises); diff --git a/src/type/copy.js b/src/type/copy.js index 0f8fa6a..9580f47 100644 --- a/src/type/copy.js +++ b/src/type/copy.js @@ -20,7 +20,10 @@ const getHashName = (file, options) => const createDirAsync = (dirPath) => { return new Promise((resolve, reject) => { mkdirp(dirPath, (err) => { - if (err) reject(err); + if (err) { + reject(err); + } + resolve(); }); }); @@ -73,7 +76,7 @@ const writeFileAsync = (file, dest) => { * @returns {Promise} */ -module.exports = function processCopy(asset, dir, options, decl, warn, addDependency) { +module.exports = function processCopy(asset, dir, options, decl, warn, result, addDependency) { if (!options.assetsPath && dir.from === dir.to) { warn('Option `to` of postcss is required, ignoring'); @@ -82,6 +85,8 @@ module.exports = function processCopy(asset, dir, options, decl, warn, addDepend return getFile(asset, options, dir, warn) .then((file) => { + if (!file) return; + const assetRelativePath = options.useHash ? getHashName(file, options.hashOptions) : asset.relativePath; diff --git a/src/type/custom.js b/src/type/custom.js index 68338dd..d274601 100644 --- a/src/type/custom.js +++ b/src/type/custom.js @@ -9,5 +9,5 @@ * @returns {Promise} */ module.exports = function getCustomProcessor(asset, dir, options) { - return options.url.apply(null, arguments); + return Promise.resolve().then(() => options.url.apply(null, arguments)); }; diff --git a/src/type/inline.js b/src/type/inline.js index f100e07..416ea97 100644 --- a/src/type/inline.js +++ b/src/type/inline.js @@ -13,7 +13,7 @@ const getFile = require('../lib/get-file'); * * @returns {String|Undefined} */ -const processFallback = (originUrl, dir, options) => { +function processFallback(originUrl, dir, options) { if (typeof options.fallback === 'function') { return options.fallback.apply(null, arguments); } @@ -25,7 +25,7 @@ const processFallback = (originUrl, dir, options) => { default: return Promise.resolve(); } -}; +} const inlineProcess = (file, asset, warn, addDependency, options) => { const isSvg = file.mimeType === 'image/svg+xml'; @@ -65,7 +65,7 @@ const inlineProcess = (file, asset, warn, addDependency, options) => { * @returns {Promise} */ // eslint-disable-next-line complexity -module.exports = function(asset, dir, options, decl, warn, addDependency) { +module.exports = function(asset, dir, options, decl, warn, result, addDependency) { return getFile(asset, options, dir, warn) .then((file) => { if (!file) return; diff --git a/src/type/rebase.js b/src/type/rebase.js index 658c2bf..9caa37e 100644 --- a/src/type/rebase.js +++ b/src/type/rebase.js @@ -21,5 +21,5 @@ module.exports = function(asset, dir, options) { path.relative(dest, asset.absolutePath) ); - return Promise.resolve(`${rebasedUrl}${asset.search}${asset.hash}`); + return Promise.resolve().then(() => `${rebasedUrl}${asset.search}${asset.hash}`); }; diff --git a/test/lib/get-file.js b/test/lib/get-file.js index 41112b5..9f04fa2 100644 --- a/test/lib/get-file.js +++ b/test/lib/get-file.js @@ -14,13 +14,15 @@ describe('get-file', () => { pathname: '../pixel.gif', absolutePath: 'test/fixtures/pixel.gif' }; - const file = getFile(asset, {}, dir, warn); - assert.deepEqual(file, { - path: 'test/fixtures/pixel.gif', - contents: fileContent, - mimeType: 'image/gif' - }); + return getFile(asset, {}, dir, warn) + .then((file) => { + assert.deepEqual(file, { + path: 'test/fixtures/pixel.gif', + contents: fileContent, + mimeType: 'image/gif' + }); + }); }); it('should show warn message when can\'t read file', () => { @@ -30,12 +32,13 @@ describe('get-file', () => { }; let warnMessage = false; - getFile(asset, {}, dir, (message) => warnMessage = message); - - assert.equal( - warnMessage, - 'Can\'t read file \'test/fixtures/pixel-no-exists.gif\', ignoring' - ); + return getFile(asset, {}, dir, (message) => warnMessage = message) + .then(() => { + assert.equal( + warnMessage, + 'Can\'t read file \'test/fixtures/pixel-no-exists.gif\', ignoring' + ); + }); }); it('should read file with basePath option', () => { @@ -44,13 +47,15 @@ describe('get-file', () => { pathname: '../pixel.gif', absolutePath: 'test/fixtures/pixel-not-exists.gif' }; - const file = getFile(asset, options, dir, warn); - assert.deepEqual(file, { - path: path.resolve('test/fixtures/pixel.gif'), - contents: fileContent, - mimeType: 'image/gif' - }); + return getFile(asset, options, dir, warn) + .then((file) => { + assert.deepEqual(file, { + path: path.resolve('test/fixtures/pixel.gif'), + contents: fileContent, + mimeType: 'image/gif' + }); + }); }); it('should read file with multiple basePath option', () => { @@ -62,12 +67,14 @@ describe('get-file', () => { pathname: '../pixel.gif', absolutePath: 'test/fixtures/pixel-not-exists.gif' }; - const file = getFile(asset, options, dir, warn); - assert.deepEqual(file, { - path: path.resolve('test/fixtures/pixel.gif'), - contents: fileContent, - mimeType: 'image/gif' - }); + return getFile(asset, options, dir, warn) + .then((file) => { + assert.deepEqual(file, { + path: path.resolve('test/fixtures/pixel.gif'), + contents: fileContent, + mimeType: 'image/gif' + }); + }); }); }); diff --git a/test/misc/messages.js b/test/misc/messages.js index de2a593..13d076e 100644 --- a/test/misc/messages.js +++ b/test/misc/messages.js @@ -13,7 +13,7 @@ describe('misc', () => { .then((result) => { const dependencies = result.messages.filter((m) => m.type === 'dependency'); - assert.deepEqual(dependencies, [ + const expected = [ { type: 'dependency', file: path.resolve('test/fixtures/imported/pixel.png'), @@ -24,7 +24,12 @@ describe('misc', () => { file: path.resolve('test/fixtures/pixel.gif'), parent: path.resolve('test/fixtures/copy.css') } - ]); + ]; + + assert.deepEqual( + dependencies.sort((a, b) => a.file.length - b.file.length), + expected.sort((a, b) => a.file.length - b.file.length) + ); }); }); @@ -40,7 +45,7 @@ describe('misc', () => { .then((result) => { const dependencies = result.messages.filter((m) => m.type === 'dependency'); - assert.deepEqual(dependencies, [ + const expected = [ { type: 'dependency', file: path.resolve('test/fixtures/imported/pixel.png'), @@ -51,7 +56,12 @@ describe('misc', () => { file: path.resolve('test/fixtures/pixel.gif'), parent: path.resolve('test/fixtures/copy.css') } - ]); + ]; + + assert.deepEqual( + dependencies.sort((a, b) => a.file.length - b.file.length), + expected.sort((a, b) => a.file.length - b.file.length) + ); }); }); }); diff --git a/test/setup.js b/test/setup.js index c759a55..47e5a6d 100644 --- a/test/setup.js +++ b/test/setup.js @@ -42,5 +42,5 @@ function processedCss(fixtures, urlOpts, postcssOpts) { return postcss() .use(url(urlOpts)) .process(read(fixtures), postcssOpts) - .css; + .then((res) => res.css); } diff --git a/test/type/copy.js b/test/type/copy.js index 24c9dec..f2e98a3 100644 --- a/test/type/copy.js +++ b/test/type/copy.js @@ -96,49 +96,54 @@ function testCopy(opts, postcssOpts) { describe('should copy asset from the source (`from`) to the assets destination (`to` + `assetsPath`)', () => { it('rebase the url', () => { - const css = processedCss('fixtures/copy', opts, postcssOpts); - - matchAll(css, ['copyPixelPng', 'copyPixelGif']); + return processedCss('fixtures/copy', opts, postcssOpts) + .then((css) => { + matchAll(css, ['copyPixelPng', 'copyPixelGif']); + }); }); it('rebase the url keeping parameters', () => { - const css = processedCss('fixtures/copy-parameters', opts, postcssOpts); - - matchAll(css, [ - 'copyParamsPixelPngHash', - 'copyParamsPixelPngParam', - 'copyParamsPixelGif' - ]); + return processedCss('fixtures/copy-parameters', opts, postcssOpts) + .then((css) => { + matchAll(css, [ + 'copyParamsPixelPngHash', + 'copyParamsPixelPngParam', + 'copyParamsPixelGif' + ]); + }); }); it('rebase the url using a hash name', () => { - const css = processedCss( + return processedCss( 'fixtures/copy-hash', optsWithHash, postcssOpts - ); - - matchAll(css, ['copyXXHashPixel8']); + ) + .then((css) => { + matchAll(css, ['copyXXHashPixel8']); + }); }); it('rebase the url using a hash name keeping parameters', () => { - const css = processedCss( + return processedCss( 'fixtures/copy-hash-parameters', optsWithHash, postcssOpts - ); - - matchAll(css, ['copyXXHashParamsPixel8']); + ) + .then((css) => { + matchAll(css, ['copyXXHashParamsPixel8']); + }); }); it('rebase the url using a hash and prepending the original filename', () => { - const css = processedCss( + return processedCss( 'fixtures/copy-hash', optsWithAppendHash, postcssOpts - ); - - matchAll(css, ['copyXXHashPrependPixel8']); + ) + .then((css) => { + matchAll(css, ['copyXXHashPrependPixel8']); + }); }); }); } diff --git a/test/type/inline.js b/test/type/inline.js index 0c3c215..96c88a2 100644 --- a/test/type/inline.js +++ b/test/type/inline.js @@ -30,29 +30,31 @@ describe('inline', () => { ); it('should inline url from dirname(from)', () => { - const css = processedCss('fixtures/inline-from', opts, postcssOpts); - - assert.ok(css.match(/;base64/)); + return processedCss('fixtures/inline-from', opts, postcssOpts) + .then((css) => { + assert.ok(css.match(/;base64/)); + }); }); it('should not inline big files from dirname(from)', () => { - const css = processedCss( + return processedCss( 'fixtures/inline-from', { url: 'inline', maxSize: 0.0001 }, { from: 'test/fixtures/here' } - ); - - assert.notOk(css.match(/;base64/)); + ).then((css) => { + assert.notOk(css.match(/;base64/)); + }); }); it('SVGs shouldn\'t be encoded in base64', () => { - const css = processedCss( + return processedCss( 'fixtures/inline-svg', { url: 'inline' }, postcssOpts - ); - - assert.notOk(css.match(/;base64/)); + ) + .then((css) => { + assert.notOk(css.match(/;base64/)); + }); }); compareFixtures( @@ -70,7 +72,7 @@ describe('inline', () => { ); it('should inline url of imported files', () => { - postcss() + return postcss() .use(require('postcss-import')()) .use(postcssUrl(opts)) .process(read('fixtures/inline-imported'), { from: 'test/fixtures/here' }) @@ -80,48 +82,49 @@ describe('inline', () => { }); it('should inline files matching the minimatch pattern', () => { - const css = processedCss( + return processedCss( 'fixtures/inline-by-type', { url: 'inline', filter: '**/*.svg' }, postcssOpts - ); - - assert.ok(css.match(/data\:image\/svg\+xml/)); - assert.notOk( - css.match(/data:image\/gif/), - 'shouldn\'t inline files not matching the minimatch pattern' - ); + ).then((css) => { + assert.ok(css.match(/data\:image\/svg\+xml/)); + assert.notOk( + css.match(/data:image\/gif/), + 'shouldn\'t inline files not matching the minimatch pattern' + ); + }); }); it('should inline files matching the regular expression', () => { - const css = processedCss( + return processedCss( 'fixtures/inline-by-type', { url: 'inline', filter: /\.svg$/ }, postcssOpts - ); - - assert.ok(css.match(/data\:image\/svg\+xml/)); - assert.notOk( - css.match(/data:image\/gif/), - 'shouldn\'t inline files not matching the regular expression' - ); + ).then((css) => { + assert.ok(css.match(/data\:image\/svg\+xml/)); + assert.notOk( + css.match(/data:image\/gif/), + 'shouldn\'t inline files not matching the regular expression' + ); + }); }); it('should inline files matching by custom function', () => { - const customFilterFunction = function(asset) { + const customFilterFunction = (asset) => { return /\.svg$/.test(asset.absolutePath); }; - const css = processedCss( + + return processedCss( 'fixtures/inline-by-type', { url: 'inline', filter: customFilterFunction }, postcssOpts - ); - - assert.ok(css.match(/data\:image\/svg\+xml/)); - assert.notOk( - css.match(/data:image\/gif/), - 'shouldn\'t inline files not matching the regular expression' - ); + ).then((css) => { + assert.ok(css.match(/data\:image\/svg\+xml/)); + assert.notOk( + css.match(/data:image\/gif/), + 'shouldn\'t inline files not matching the regular expression' + ); + }); }); describe('function when inline fallback', () => { @@ -151,7 +154,7 @@ describe('inline', () => { }); it('should find files in basePaths', () => { - const css = processedCss( + return processedCss( 'fixtures/inline-by-base-paths', { url: 'inline', @@ -159,8 +162,8 @@ describe('inline', () => { basePath: [path.resolve('test/fixtures/baseDir1'), 'baseDir2'] }, postcssOpts - ); - - assert.equal(css.match(/data:image\/png/g).length, 2); + ).then((css) => { + assert.equal(css.match(/data:image\/png/g).length, 2); + }); }); }); diff --git a/test/type/rebase.js b/test/type/rebase.js index 643dd6d..098b8d9 100644 --- a/test/type/rebase.js +++ b/test/type/rebase.js @@ -5,19 +5,19 @@ describe('rebase', () => { describe('base unit', () => { it('should calc relative path', () => { - const res = rebase({ + return rebase({ absolutePath: '/project/blocks/item/1.png', search: '', hash: '' }, { to: '/project/build' + }).then((res) => { + assert.equal(res, '../blocks/item/1.png'); }); - - assert.equal(res, '../blocks/item/1.png'); }); it('should calc relative path by assetsPath option', () => { - const res = rebase({ + return rebase({ absolutePath: '/project/blocks/item/1.png', search: '', hash: '' @@ -27,18 +27,21 @@ describe('rebase', () => { from: '/project/blocks/item/1.png' }, { assetsPath: '/project/build' - }); - - assert.equal(res, '../blocks/item/1.png'); + }) + .then((res) => { + assert.equal(res, '../blocks/item/1.png'); + }); }); }); it('rebase with empty options', () => { - processedCss( + return processedCss( 'fixtures/copy-hash', undefined, { from: 'test/fixtures/here' } - ).css; + ).then((css) => { + assert(css); + }); }); compareFixtures( From 3f3dddf3466f8a89d4a7d57754f2ccd6818768a8 Mon Sep 17 00:00:00 2001 From: Konstantin Mamaev Date: Sat, 6 Apr 2019 13:35:58 +0300 Subject: [PATCH 4/4] Fix comment issues --- src/lib/get-file.js | 51 ++++++++++++++++++++++++++------------------- src/type/copy.js | 25 ++++------------------ 2 files changed, 34 insertions(+), 42 deletions(-) diff --git a/src/lib/get-file.js b/src/lib/get-file.js index c9d718f..8da6c34 100644 --- a/src/lib/get-file.js +++ b/src/lib/get-file.js @@ -17,26 +17,32 @@ const readFileAsync = (filePath) => { }; const existFileAsync = (filePath) => { - return new Promise((resolve, reject) => + return new Promise((resolve) => fs.access(filePath, (err) => { - if (err) { - reject(); - } - resolve(filePath); + resolve(!err); }) ); }; -const oneSuccess = (promises) => { - return Promise.all(promises.map((p) => { - return p.then( - (val) => Promise.reject(val), - (err) => Promise.resolve(err) - ); - })).then( - (errors) => Promise.reject(errors), - (val) => Promise.resolve(val) - ); +const findExistsPath = (paths) => { + let resolved = false; + + return new Promise((resolve, reject) => { + const findPromises = paths.map((path) => { + return existFileAsync(path).then((isExists) => { + if (!resolved && isExists) { + resolved = true; + resolve(path); + } + }); + }); + + Promise.all(findPromises).then(() => { + if (!resolved) { + reject(); + } + }); + }); }; /** @@ -52,13 +58,16 @@ const getFile = (asset, options, dir, warn) => { ? getPathByBasePath(options.basePath, dir.from, asset.pathname) : [asset.absolutePath]; - return oneSuccess(paths.map((path) => existFileAsync(path))) + return findExistsPath(paths) .then((path) => readFileAsync(path) - .then((contents) => ({ - path, - contents, - mimeType: mime.getType(path) - }))) + .then((contents) => { + return { + path, + contents, + mimeType: mime.getType(path) + }; + }) + ) .catch(() => { warn(`Can't read file '${paths.join()}', ignoring`); diff --git a/src/type/copy.js b/src/type/copy.js index 9580f47..5eb7bb0 100644 --- a/src/type/copy.js +++ b/src/type/copy.js @@ -31,30 +31,13 @@ const createDirAsync = (dirPath) => { const writeFileAsync = (file, dest) => { return new Promise((resolve, reject) => { - fs.open(dest, 'wx', (err, fd) => { + fs.writeFile(dest, file.contents, { flag: 'wx' }, (err) => { if (err) { - if (err.code === 'EEXIST') { - resolve(); - } - - reject(err); + err.code === 'EEXIST' ? resolve() : reject(err); } - - resolve(fd); - }); - }) - .then((fd) => { - if (!fd) return; - - return new Promise((resolve, reject) => { - fs.writeFile(dest, file.contents, (err) => { - if (err) { - reject(err); - } - resolve(); - }); - }); + resolve(); }); + }); }; /**