Skip to content

Commit

Permalink
fix: Overlapping value replacement (#365)
Browse files Browse the repository at this point in the history
* WIP: sort values to match by length

So that values with the same prefixes don't stomp over each other. This feels a bit hacky but works for now.

Fixes #363

* test: Fix up travis failures

- Directly write files instead of copying
- Use travis in VM mode instead of container
  • Loading branch information
tivac authored Nov 13, 2017
1 parent a2f5f58 commit 1f6fdb5
Show file tree
Hide file tree
Showing 11 changed files with 84 additions and 58 deletions.
6 changes: 3 additions & 3 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
sudo: false
# Force a VM instead of Docker, some tests that depend on the FS fail otherwise
sudo: required

language: node_js
node_js:
- node
- '7'
- '6'
- '8'

# https://docs.travis-ci.com/user/customizing-the-build/#Fast-Finishing
matrix:
Expand Down
40 changes: 20 additions & 20 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions packages/browserify/test/__snapshots__/issue-58.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,5 @@ exports[`/browserify.js /issues /58 should update when CSS dependencies change 3
}
.mc46e4a65d_issue2 {
color: aqua;
}
"
}"
`;
73 changes: 49 additions & 24 deletions packages/browserify/test/issue-58.test.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,26 @@
"use strict";

var from = require("from2-string"),
var fs = require("fs"),

from = require("from2-string"),
shell = require("shelljs"),
browserify = require("browserify"),
watchify = require("watchify"),
dedent = require("dedent"),

read = require("test-utils/read.js")(__dirname),

bundle = require("./lib/bundle.js"),
plugin = require("../browserify.js");

function write(txt) {
fs.writeFileSync(
"./packages/browserify/test/specimens/issues/58/other.css",
dedent(txt),
"utf8"
);
}

describe("/browserify.js", () => {
describe("/issues", () => {
describe("/58", () => {
Expand All @@ -21,41 +32,55 @@ describe("/browserify.js", () => {
it("should update when CSS dependencies change", (done) => {
var build = browserify();

shell.cp("-f",
"./packages/browserify/test/specimens/issues/58/1.css",
"./packages/browserify/test/specimens/issues/58/other.css"
);
write(`
.other1 { color: red; }
.other2 { color: navy; }
.other3 { color: blue; }
`);

build.add(from("require('./packages/browserify/test/specimens/issues/58/issue.css');"));
build.add(
from("require('./packages/browserify/test/specimens/issues/58/issue.css');")
);

build.plugin(watchify);
build.plugin(plugin, {
css : "./packages/browserify/test/output/issues/58.css"
});

build.on("update", () => {
bundle(build)
.then((out) => {
expect(out).toMatchSnapshot();

expect(read("./issues/58.css")).toMatchSnapshot();

build.close();

done();
});
});
console.log("File change noticed");

bundle(build).then((out) => {
console.log("second build complete");

bundle(build)
.then((out) => {
expect(out).toMatchSnapshot();

expect(read("./issues/58.css")).toMatchSnapshot();

console.log("snapshot tests complete");

build.close();

shell.cp("-f",
"./packages/browserify/test/specimens/issues/58/2.css",
"./packages/browserify/test/specimens/issues/58/other.css"
);
console.log("test finished");

done();
});
}, 10000);
});

bundle(build).then((out) => {
console.log("initial build complete");

expect(out).toMatchSnapshot();

write(`
.other1 { color: green; }
.other2 { color: yellow; }
.other3 { composes: other2; background: white; }
`);

console.log("File overwritten");
});
});
});
});
});
3 changes: 0 additions & 3 deletions packages/browserify/test/specimens/issues/58/1.css

This file was deleted.

3 changes: 0 additions & 3 deletions packages/browserify/test/specimens/issues/58/2.css

This file was deleted.

3 changes: 0 additions & 3 deletions packages/cli/test/cli.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@ function success(out) {
}

describe("/cli.js", function() {
// Since these tests keep failing on Travis...
jest.setTimeout(10000);

afterAll(() => require("shelljs").rm("-rf", "./packages/cli/test/output"));

it("should show help with no args", () =>
Expand Down
1 change: 1 addition & 0 deletions packages/core/plugins/values-replace.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ module.exports = (css, result) => {

matchRegex = new RegExp(
Object.keys(values)
.sort((a, b) => b.length - a.length)
.map((v) => `\\b${escape(v)}\\b`)
.join("|"),
"g"
Expand Down
4 changes: 4 additions & 0 deletions packages/core/test/__snapshots__/values.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,10 @@ exports[`/processor.js values should support value namespaces 1`] = `
.blue {
color: blue;
}
.other {
color: #000;
}
"
`;

Expand Down
4 changes: 4 additions & 0 deletions packages/core/test/specimens/value-namespace.css
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,7 @@
.blue {
color: colors.b;
}

.other {
color: colors.base-other;
}
2 changes: 2 additions & 0 deletions packages/core/test/specimens/values.css
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
@value a: red;
@value b: blue;
@value base: #FFF;
@value base-other: #000;

0 comments on commit 1f6fdb5

Please sign in to comment.