From ba3dcf265ac3115a6db08a9000a966bb686e78f3 Mon Sep 17 00:00:00 2001 From: Bogdan Chadkin Date: Tue, 22 Dec 2015 21:19:42 +0300 Subject: [PATCH 1/5] Prepare async path resolving --- index.js | 126 +++++++++--------------------- lib/resolve-id.js | 54 +++++++++++++ test/fixtures/ignore.expected.css | 7 +- test/import.js | 29 +++---- 4 files changed, 110 insertions(+), 106 deletions(-) create mode 100644 lib/resolve-id.js diff --git a/index.js b/index.js index c30624d9..cd1d3ec5 100755 --- a/index.js +++ b/index.js @@ -5,12 +5,12 @@ var fs = require("fs") var path = require("path") var assign = require("object-assign") -var resolve = require("resolve") var postcss = require("postcss") var helpers = require("postcss-message-helpers") var glob = require("glob") var parseImports = require("./lib/parse-imports") var resolveMedia = require("./lib/resolve-media") +var resolveId = require("./lib/resolve-id") /** * Constants @@ -18,7 +18,6 @@ var resolveMedia = require("./lib/resolve-media") var moduleDirectories = [ "web_modules", "node_modules", - "bower_components", ] /** @@ -256,42 +255,48 @@ function readAtImport( return Promise.resolve() } + var dir = atRule.source && atRule.source.input && atRule.source.input.file + ? path.dirname(path.resolve(options.root, atRule.source.input.file)) + : options.root + addInputToPath(options) - var resolvedFilename = resolveFilename( - parsedAtImport.uri, - options.root, - options.path, - atRule.source, - options.resolve - ) - if (options.skipDuplicates) { - // skip files already imported at the same scope - if ( - state.importedFiles[resolvedFilename] && - state.importedFiles[resolvedFilename][media] - ) { - atRule.remove() - return Promise.resolve() + return Promise.resolve().then(function() { + if (options.resolve) { + return options.resolve(parsedAtImport.uri, dir, options) } + return resolveId(parsedAtImport.uri, dir, options.path) + }).then(function(resolvedFilename) { + if (options.skipDuplicates) { + // skip files already imported at the same scope + if ( + state.importedFiles[resolvedFilename] && + state.importedFiles[resolvedFilename][media] + ) { + atRule.remove() + return Promise.resolve() + } - // save imported files to skip them next time - if (!state.importedFiles[resolvedFilename]) { - state.importedFiles[resolvedFilename] = {} + // save imported files to skip them next time + if (!state.importedFiles[resolvedFilename]) { + state.importedFiles[resolvedFilename] = {} + } + state.importedFiles[resolvedFilename][media] = true } - state.importedFiles[resolvedFilename][media] = true - } - return readImportedContent( - result, - atRule, - parsedAtImport, - assign({}, options), - resolvedFilename, - state, - media, - processor - ) + return readImportedContent( + result, + atRule, + parsedAtImport, + assign({}, options), + resolvedFilename, + state, + media, + processor + ) + }).catch(function(err) { + result.warn(err.message, { node: atRule }) + }) } /** @@ -414,63 +419,6 @@ function insertRules(atRule, parsedAtImport, newStyles) { atRule.replaceWith(newNodes) } -/** - * Check if a file exists - * - * @param {String} name - */ -function resolveFilename(name, root, paths, source, resolver) { - var dir = source && source.input && source.input.file - ? path.dirname(path.resolve(root, source.input.file)) - : root - - try { - var resolveOpts = { - basedir: dir, - moduleDirectory: moduleDirectories.concat(paths), - paths: paths, - extensions: [ ".css" ], - packageFilter: function processPackage(pkg) { - pkg.main = pkg.style || "index.css" - return pkg - }, - } - var file - resolver = resolver || resolve.sync - try { - file = resolver(name, resolveOpts) - } - catch (e) { - // fix to try relative files on windows with "./" - // if it's look like it doesn't start with a relative path already - // if (name.match(/^\.\.?/)) {throw e} - try { - file = resolver("./" + name, resolveOpts) - } - catch (err) { - // LAST HOPE - if (!paths.some(function(dir2) { - file = path.join(dir2, name) - return fs.existsSync(file) - })) { - throw err - } - } - } - - return path.normalize(file) - } - catch (e) { - throw new Error( - "Failed to find '" + name + "' from " + root + - "\n in [ " + - "\n " + paths.join(",\n ") + - "\n ]", - source - ) - } -} - /** * Read the contents of a file * diff --git a/lib/resolve-id.js b/lib/resolve-id.js new file mode 100644 index 00000000..4081ccdb --- /dev/null +++ b/lib/resolve-id.js @@ -0,0 +1,54 @@ +var fs = require("fs") +var path = require("path") +var resolve = require("resolve") + +var moduleDirectories = [ + "web_modules", + "node_modules", +] + +module.exports = function(id, base, paths) { + try { + var resolveOpts = { + basedir: base, + moduleDirectory: moduleDirectories, + paths: paths, + extensions: [ ".css" ], + packageFilter: function processPackage(pkg) { + pkg.main = pkg.style || "index.css" + return pkg + }, + } + var file + try { + file = resolve.sync(id, resolveOpts) + } + catch (e) { + // fix to try relative files on windows with "./" + // if it's look like it doesn't start with a relative path already + // if (id.match(/^\.\.?/)) {throw e} + try { + file = resolve.sync("./" + id, resolveOpts) + } + catch (err) { + // LAST HOPE + if (!paths.some(function(dir) { + file = path.join(dir, id) + return fs.existsSync(file) + })) { + throw err + } + } + } + + return path.normalize(file) + } + catch (e) { + throw new Error([ + "Failed to find '" + id + "'", + "in [ ", + " " + paths.join(",\n "), + "]", + ].join("\n ")) + } +} diff --git a/test/fixtures/ignore.expected.css b/test/fixtures/ignore.expected.css index 804b9950..9187131a 100644 --- a/test/fixtures/ignore.expected.css +++ b/test/fixtures/ignore.expected.css @@ -1,6 +1,3 @@ -@import "http://css" (min-width: 25em); - -@import "http://css-screen" (min-width: 25em) and screen; @import "http://css"; @@ -28,6 +25,10 @@ @import url(//css); +@import "http://css" (min-width: 25em); + +@import "http://css-screen" (min-width: 25em) and screen; + @media (min-width: 25em){ ignore{} } diff --git a/test/import.js b/test/import.js index 4dccc1cb..58179b17 100644 --- a/test/import.js +++ b/test/import.js @@ -1,4 +1,5 @@ import test from "ava" +import path from "path" import { readFileSync } from "fs" import postcss from "postcss" import atImport from ".." @@ -85,13 +86,11 @@ test("should output readable trace", t => { return postcss() .use(atImport()) .process(readFileSync(file), { from: file }) - .catch(error => { - t.throws( - () => { - throw error - }, + .then(result => { + t.is( + result.warnings()[0].text, /* eslint-disable max-len */ - /import-missing.css:2:5: Failed to find 'missing-file.css' from .*\n\s+in \[/gm + "Failed to find 'missing-file.css'\n in [ \n " + path.resolve("fixtures/imports") + "\n ]" /* eslint-enabme max-len */ ) }) @@ -155,14 +154,16 @@ test("should work with no styles without throwing an error", t => { test("should be able to consume modules in the custom-resolve way", t => { const resolve = require("resolve") - const sassResolve = (file, opts) => { - opts = opts || {} - opts.extensions = [ ".scss", ".css" ] - opts.packageFilter = pkg => { - pkg.main = pkg.sass || pkg.style || "index" - return pkg - } - return resolve.sync(file, opts) + const sassResolve = (id, base, opts) => { + return resolve.sync(id, { + basedir: base, + extensions: [ ".scss", ".css" ], + paths: opts.path, + packageFilter: pkg => { + pkg.main = pkg.sass || pkg.style || "index" + return pkg + }, + }) } return compareFixtures(t, "custom-resolve-modules", { root: ".", From 576d3b91e50b5aee672edc6985f11da9cf4ff861 Mon Sep 17 00:00:00 2001 From: Bogdan Chadkin Date: Wed, 23 Dec 2015 00:33:46 +0300 Subject: [PATCH 2/5] Remove message-helpers dep --- index.js | 21 +++++++++------------ package.json | 1 - 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/index.js b/index.js index cd1d3ec5..4e140628 100755 --- a/index.js +++ b/index.js @@ -6,7 +6,6 @@ var path = require("path") var assign = require("object-assign") var postcss = require("postcss") -var helpers = require("postcss-message-helpers") var glob = require("glob") var parseImports = require("./lib/parse-imports") var resolveMedia = require("./lib/resolve-media") @@ -140,17 +139,15 @@ function parseStyles( }) var importResults = imports.map(function(instance) { - return helpers.try(function transformAtImport() { - return readAtImport( - result, - instance.node, - instance, - options, - state, - media, - processor - ) - }, instance.node.source) + return readAtImport( + result, + instance.node, + instance, + options, + state, + media, + processor + ) }) return Promise.all(importResults) diff --git a/package.json b/package.json index 0045f9bd..979e5200 100644 --- a/package.json +++ b/package.json @@ -20,7 +20,6 @@ "glob": "^5.0.14", "object-assign": "^4.0.1", "postcss": "^5.0.2", - "postcss-message-helpers": "^2.0.0", "postcss-value-parser": "^3.2.3", "resolve": "^1.1.6" }, From c4c83edf2da1509d6b04b021b8ccab16a051fac4 Mon Sep 17 00:00:00 2001 From: Bogdan Chadkin Date: Wed, 23 Dec 2015 14:01:01 +0300 Subject: [PATCH 3/5] Fix ignored rules order with async --- index.js | 49 +++++++++++++++++++------------ test/fixtures/ignore.expected.css | 7 ++--- 2 files changed, 34 insertions(+), 22 deletions(-) diff --git a/index.js b/index.js index 4e140628..ab49baf1 100755 --- a/index.js +++ b/index.js @@ -65,7 +65,6 @@ function AtImport(options) { var state = { importedFiles: {}, - ignoredAtRules: [], hashFiles: {}, } if (opts.from) { @@ -83,8 +82,8 @@ function AtImport(options) { createProcessor(result, options.plugins) ) - function onParseEnd() { - addIgnoredAtRulesOnTop(styles, state.ignoredAtRules) + function onParseEnd(ignored) { + addIgnoredAtRulesOnTop(styles, ignored) if ( typeof opts.addDependencyTo === "object" && @@ -150,7 +149,21 @@ function parseStyles( ) }) - return Promise.all(importResults) + return Promise.all(importResults).then(function(result) { + // Flatten ignored instances + return result.reduce(function(ignored, item) { + if (Array.isArray(item)) { + item = item.filter(function(instance) { + return instance + }) + ignored = ignored.concat(item) + } + else if (item) { + ignored.push(item) + } + return ignored + }, []) + }) } /** @@ -208,15 +221,14 @@ function parseGlob(imports, instance, options) { * @param {Array} state */ function addIgnoredAtRulesOnTop(styles, ignoredAtRules) { - var i = ignoredAtRules.length - if (i) { - while (i--) { - var ignored = ignoredAtRules[i][1] - ignored.node.params = ignored.fullUri + - (ignored.media.length ? " " + ignored.media.join(", ") : "") - - styles.prepend(ignored.node) - } + var i = ignoredAtRules.length - 1 + while (i !== -1) { + var ignored = ignoredAtRules[i] + ignored.node.params = ignored.fullUri + + (ignored.media.length ? " " + ignored.media.join(", ") : "") + + styles.prepend(ignored.node) + i -= 1 } } @@ -243,13 +255,10 @@ function readAtImport( if (parsedAtImport.uri.match(/^(?:[a-z]+:)?\/\//i)) { parsedAtImport.media = media - // save - state.ignoredAtRules.push([ atRule, parsedAtImport ]) - // detach atRule.remove() - return Promise.resolve() + return Promise.resolve(parsedAtImport) } var dir = atRule.source && atRule.source.input && atRule.source.input.file @@ -370,7 +379,10 @@ function readImportedContent( processor ) - return parsedResult.then(function() { + var instances + + return parsedResult.then(function(result) { + instances = result return processor.process(newStyles) }) .then(function(newResult) { @@ -378,6 +390,7 @@ function readImportedContent( }) .then(function() { insertRules(atRule, parsedAtImport, newStyles) + return instances }) } diff --git a/test/fixtures/ignore.expected.css b/test/fixtures/ignore.expected.css index 9187131a..804b9950 100644 --- a/test/fixtures/ignore.expected.css +++ b/test/fixtures/ignore.expected.css @@ -1,3 +1,6 @@ +@import "http://css" (min-width: 25em); + +@import "http://css-screen" (min-width: 25em) and screen; @import "http://css"; @@ -25,10 +28,6 @@ @import url(//css); -@import "http://css" (min-width: 25em); - -@import "http://css-screen" (min-width: 25em) and screen; - @media (min-width: 25em){ ignore{} } From eeb3266ad118c96b4903742ba217575b81fcc6bd Mon Sep 17 00:00:00 2001 From: Bogdan Chadkin Date: Thu, 24 Dec 2015 10:58:57 +0300 Subject: [PATCH 4/5] Update documentation and changelog --- CHANGELOG.md | 5 +++++ README.md | 6 ++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 86ace66d..5ce336ad 100755 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ - Removed: async mode/option ([#107](https://github.com/postcss/postcss-import/pull/107)) +- Removed: "bower_components" not supported by default anymore, use "path" option to add it back +([#116](https://github.com/postcss/postcss-import/pull/116)) +- Changed: custom resolve has more responsibility for paths resolving. +See [resolve option](https://github.com/postcss/postcss-import#resolve) for more information about this change +([#116](https://github.com/postcss/postcss-import/pull/116)) # 7.1.3 - 2015-11-05 diff --git a/README.md b/README.md index 80078979..d37171af 100755 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@ This plugin can consume local files, node modules or bower packages. To resolve path of an `@import` rule, it can look into root directory -(by default `process.cwd()`), `web_modules`, `node_modules`, `bower_components` +(by default `process.cwd()`), `web_modules`, `node_modules` or local modules. _When importing a module, it will looks for `index.css` or file referenced in `package.json` in the `style` field._ @@ -144,7 +144,9 @@ Set to `true` if you want @import rules to parse glob patterns. Type: `Function` Default: `null` -You can overwrite the default path resolving way by setting this option, using the `resolve.sync(id, opts)` signature that [resolve.sync](https://github.com/substack/node-resolve#resolvesyncid-opts) has. +You can overwrite the default path resolving way by setting this option. +This function gets `(id, basedir, importOptions)` arguments and returns full path or promise resolving full path. +You can use [resolve](https://github.com/substack/node-resolve) for that. #### `skipDuplicates` From 9c32c3c4018f0cbe01366133655cb40451ca3eb2 Mon Sep 17 00:00:00 2001 From: Bogdan Chadkin Date: Wed, 23 Dec 2015 16:21:38 +0300 Subject: [PATCH 5/5] Promisify resolve-id --- lib/resolve-id.js | 96 +++++++++++++++++++++++++++-------------------- 1 file changed, 56 insertions(+), 40 deletions(-) diff --git a/lib/resolve-id.js b/lib/resolve-id.js index 4081ccdb..11121d8b 100644 --- a/lib/resolve-id.js +++ b/lib/resolve-id.js @@ -7,48 +7,64 @@ var moduleDirectories = [ "node_modules", ] -module.exports = function(id, base, paths) { - try { - var resolveOpts = { - basedir: base, - moduleDirectory: moduleDirectories, - paths: paths, - extensions: [ ".css" ], - packageFilter: function processPackage(pkg) { - pkg.main = pkg.style || "index.css" - return pkg - }, - } - var file - try { - file = resolve.sync(id, resolveOpts) - } - catch (e) { - // fix to try relative files on windows with "./" - // if it's look like it doesn't start with a relative path already - // if (id.match(/^\.\.?/)) {throw e} - try { - file = resolve.sync("./" + id, resolveOpts) - } - catch (err) { - // LAST HOPE - if (!paths.some(function(dir) { - file = path.join(dir, id) - return fs.existsSync(file) - })) { - throw err +function isFilePromise(path) { + return new Promise(function(resolve, reject) { + fs.stat(path, function(err, stat) { + if (err) { + if (err.code === "ENOENT") { + return resolve(false) } + return reject(err) } - } + resolve(stat.isFile() || stat.isFIFO()) + }) + }) +} - return path.normalize(file) - } - catch (e) { - throw new Error([ - "Failed to find '" + id + "'", - "in [ ", - " " + paths.join(",\n "), - "]", - ].join("\n ")) +function resolvePromise(id, opts) { + return new Promise(function(res, rej) { + resolve(id, opts, function(err, path) { + if (err) { + return rej(err) + } + res(path) + }) + }) +} + +module.exports = function(id, base, paths) { + var resolveOpts = { + basedir: base, + moduleDirectory: moduleDirectories, + paths: paths, + extensions: [ ".css" ], + packageFilter: function processPackage(pkg) { + pkg.main = pkg.style || "index.css" + return pkg + }, } + return resolvePromise(id, resolveOpts).catch(function() { + // fix to try relative files on windows with "./" + // if it's look like it doesn't start with a relative path already + // if (id.match(/^\.\.?/)) {throw e} + return resolvePromise("./" + id, resolveOpts) + }).catch(function() { + // LAST HOPE + return Promise.all(paths.map(function(p) { + return isFilePromise(path.resolve(p, id)) + })).then(function(results) { + for (var i = 0; i < results.length; i += 1) { + if (results[i]) { + return path.resolve(paths[i], id) + } + } + + throw new Error([ + "Failed to find '" + id + "'", + "in [ ", + " " + paths.join(",\n "), + "]", + ].join("\n ")) + }) + }) }