Skip to content
Permalink
Browse files

refactor(processor): add dupewarn option and no longer resolve path c…

…ase (#582)

BREAKING CHANGE:

It was causing massive slowdowns to synchronously resolve files using `true-case-path`, and making it async wasn't a guaranteed win either. So it's removed, which should solve #581 pretty neatly.

Instead now there's a warning if two files are included that differ in case only. It can be disabled by setting `dupewarn : false` as part of the config object.
  • Loading branch information...
tivac committed May 8, 2019
1 parent a06ec9b commit 01581f9e5cb8f49e0007aff27f051e1186922dfb

Large diffs are not rendered by default.

Oops, something went wrong.
@@ -30,26 +30,26 @@
"cssnano": "^4.1.10",
"dedent": "0.7.0",
"dentist": "1.0.3",
"env-cmd": "^8.0.2",
"env-cmd": "^9.0.1",
"eslint": "^5.16.0",
"eslint-plugin-jest": "^22.4.1",
"eslint-plugin-jest": "^22.5.1",
"factor-bundle": "2.5.0",
"from2-string": "1.1.0",
"jest": "^24.7.0",
"jest-cli": "^24.7.0",
"jest": "^24.8.0",
"jest-cli": "^24.8.0",
"jest-runner-eslint": "^0.7.3",
"lerna": "^3.13.1",
"lerna": "^3.13.4",
"pegjs": "0.10.0",
"read-dir-deep": "^4.0.3",
"rollup": "^1.7.4",
"read-dir-deep": "^5.0.0",
"rollup": "^1.11.3",
"rollup-plugin-hypothetical": "^2.1.0",
"rollup-plugin-svelte": "^5.0.3",
"shelljs": "^0.8.3",
"snapshot-diff": "^0.5.1",
"sugarss": "^2.0.0",
"svelte": "^2.16.1",
"watchify": "^3.11.1",
"webpack": "^4.29.6"
"webpack": "^4.30.0"
},
"dependencies": {
"@modular-css/browserify": "file:packages/browserify",
@@ -29,7 +29,7 @@
"dependencies": {
"@modular-css/processor": "file:../processor",
"mkdirp": "^0.5.1",
"p-each-series": "^1.0.0",
"p-each-series": "^2.0.0",
"sink-transform": "^2.0.0",
"through2": "^2.0.3"
}
@@ -1,16 +1,11 @@
"use strict";

const path = require("path");
const filecase = require("true-case-path");

module.exports = (cwd, file) => {
if(!path.isAbsolute(file)) {
file = path.join(cwd, file);
}

file = path.normalize(file);

// If the file doesn't exist filecase() returns undefined, so in that
// instance just use whatever was sent
return filecase(file) || file;

return path.normalize(file);
};
@@ -2,7 +2,6 @@

const path = require("path");
const resolve = require("resolve-from").silent;
const pathcase = require("true-case-path");

exports.resolve = (src, file) => resolve(path.dirname(src), file);

@@ -16,10 +15,6 @@ exports.resolvers = (resolvers) => {
(result = fn(src, file, exports.resolve))
);

if(!result) {
throw new Error(`Unable to locate "${file}" from "${src}"`);
}

return pathcase(result);
return result;
};
};
@@ -25,14 +25,13 @@
],
"dependencies": {
"dependency-graph": "^0.8.0",
"escape-string-regexp": "^1.0.5",
"escape-string-regexp": "^2.0.0",
"lodash": "^4.17.0",
"postcss": "^7.0.0",
"postcss-selector-parser": "^6.0.2",
"postcss-url": "^8.0.0",
"postcss-value-parser": "^3.3.0",
"resolve-from": "^4.0.0",
"true-case-path": "^1.0.2",
"resolve-from": "^5.0.0",
"unique-slug": "^2.0.0"
},
"peerDependencies": {
@@ -43,6 +43,7 @@ class Processor {
verbose : false,
resolvers : [],
postcss : {},
dupewarn : true,
},
opts
);
@@ -74,6 +75,7 @@ class Processor {

this._files = Object.create(null);
this._graph = new Graph();
this._ids = new Map();

this._before = postcss([
...(options.before || []),
@@ -174,10 +176,12 @@ class Processor {

const deps = this.dependents(source);

deps.concat(source).forEach((file) => {
[ ...deps, source ].forEach((file) => {
this._log("invalidate()", file);

this._files[file].valid = false;

this._ids.delete(file.toLowerCase());
});
}

@@ -280,7 +284,7 @@ class Processor {
),
});

root.append([ comment ].concat(result.root.nodes));
root.append([ comment, ...result.root.nodes ]);

const idx = root.index(comment);

@@ -333,11 +337,24 @@ class Processor {
// Take a file id and some text, walk it for dependencies, then
// process and return details
async _add(id, text) {
const check = id.toLowerCase();

// Warn about potential dupes if an ID goes past we've seen before
if(this._options.dupewarn) {
const other = this._ids.get(check);

if(other && other !== id) {
console.warn(`POTENTIAL DUPLICATE FILES:\n\t${relative(this._options.cwd, other)}\n\t${relative(this._options.cwd, id)}`);
}
}

this._ids.set(check, id);

this._log("_add()", id);

await this._walk(id, text);

const deps = this._graph.dependenciesOf(id).concat(id);
const deps = [ ...this._graph.dependenciesOf(id), id ];

for(const dep of deps) {
const file = this._files[dep];
@@ -57,6 +57,16 @@ exports[`/processor.js options lifecycle options done should run sync postcss pl
a {}"
`;

exports[`/processor.js options lifecycle options dupewarn should warn on potentially duplicate file paths 1`] = `
Array [
Array [
"POTENTIAL DUPLICATE FILES:
packages/processor/test/specimens/start.css
packages/processor/test/specimens/START.css",
],
]
`;

exports[`/processor.js options lifecycle options processing should include exports from 'modular-css-export' modules 1`] = `
Object {
"a": true,
@@ -58,13 +58,13 @@ describe("/processor.js", () => {
],
});

const file = require.resolve("./specimens/composes.css");

try {
await processor.file(file);
} catch({ message }) {
await processor.file(require.resolve("./specimens/composes.css"));
} catch(e) {
const { message } = e;

expect(message).toMatch(
`Unable to locate "./folder/folder2.css" from "${file}"`
`no such file or directory, open '${require.resolve("./specimens/folder/folder2.css")}a'`
);
}
});

This file was deleted.

Oops, something went wrong.

This file was deleted.

Oops, something went wrong.

This file was deleted.

Oops, something went wrong.

This file was deleted.

Oops, something went wrong.
Oops, something went wrong.

0 comments on commit 01581f9

Please sign in to comment.
You can’t perform that action at this time.