Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(processor): allow composes anywhere in a rule #646

Merged
merged 3 commits into from
Aug 12, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 0 additions & 18 deletions packages/processor/plugins/composition.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,6 @@ const parser = require("../parsers/composes.js");

const plugin = "modular-css-composition";

// Loop through all previous nodes in the container to ensure
// that composes (or a comment) comes first
const composesFirst = (decl) => {
let prev = decl.prev();

while(prev) {
if(prev.type !== "comment") {
throw decl.error("composes must be the first declaration", {
word : "composes",
});
}

prev = prev.prev();
}
};

module.exports = (css, result) => {
const { opts } = result;

Expand All @@ -47,8 +31,6 @@ module.exports = (css, result) => {
const details = parser.parse(decl.value);
const selectors = decl.parent.selectors.map(identifiers.parse);

composesFirst(decl);

// https://github.com/tivac/modular-css/issues/238
if(selectors.some(({ length }) => length > 1)) {
throw decl.error(
Expand Down
10 changes: 10 additions & 0 deletions packages/processor/test/__snapshots__/composition.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,16 @@ Object {
}
`;

exports[`/processor.js composition should allow composes anywhere 1`] = `
Object {
"multiple-composes.css": Object {
"a": "a",
"b": "a b",
"c": "a b c",
},
}
`;

exports[`/processor.js composition should compose a single class 1`] = `
Object {
"single-composes.css": Object {
Expand Down
40 changes: 23 additions & 17 deletions packages/processor/test/composition.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,23 +69,6 @@ describe("/processor.js", () => {
}
});

it("should fail if composes isn't the first property", async () => {
try {
await processor.string(
"./invalid/composes-first.css",
dedent(`
.a { color: red; }
.b {
color: blue;
composes: a;
}
`)
);
} catch({ message }) {
expect(message).toMatch(`composes must be the first declaration`);
}
});

it("should fail on rules that use multiple selectors", async () => {
try {
await processor.string(
Expand Down Expand Up @@ -130,6 +113,29 @@ describe("/processor.js", () => {

expect(compositions).toMatchSnapshot();
});

it("should allow composes anywhere", async () => {
await processor.string(
"./multiple-composes.css",
dedent(`
.a { color: red; }
.b {
background: blue;
composes: a;
}
.c {
border: 1px solid red;
composes: a;
text-weight: bold;
composes: b;
}
`)
);

const { compositions } = await processor.output();

expect(compositions).toMatchSnapshot();
});

it("should compose from globals", async () => {
await processor.string(
Expand Down
4 changes: 4 additions & 0 deletions packages/www/src/guide/comparison.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,3 +93,7 @@ CSS Modules allows for importing multiple values from an external file.
### Support

CSS Modules is really only fully-supported as part of webpack these days. The repo is effectively mothballed, and there appears to be almost no one supporting the current codebase. Since webpack is the only live version & we prefer to use rollup for it's cleaner output and smaller bundles we needed an approach that could be flexibly be used with a [variety](#rollup) [of](#browserify) [tools](#cli).

### `composes` location

After a long time with `composes` being required to be the first declaration in a rule the ability to use a tool like [`stylelint-order`](https://github.com/hudochenkov/stylelint-order) has reduced the need for `modular-css` to enforce positional requirements. After a push in [#645](https://github.com/tivac/modular-css/issues/645) the requirement was removed in [#646](https://github.com/tivac/modular-css/pull/646), which was published in v25. This is a change from CSS Modules, which continues to require that `composes` be the first declaration in a rule.