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

Expand variables in AtRule properties #104

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 6 additions & 3 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,10 @@ module.exports = postcss.plugin('postcss-css-variables', function(options) {

// Collect all the rules that have declarations that use variables
var rulesThatHaveDeclarationsWithVariablesList = [];
css.walkRules(function(rule) {
css.walk(function(rule) {
// We're only interested in Containers with children.
if (rule.nodes === undefined) return;

var doesRuleUseVariables = rule.nodes.some(function(node) {
if(node.type === 'decl') {
var decl = node;
Expand All @@ -234,8 +237,8 @@ module.exports = postcss.plugin('postcss-css-variables', function(options) {
rulesThatHaveDeclarationsWithVariablesList.forEach(function(rule) {
var rulesToWorkOn = [].concat(rule);
// Split out the rule into each comma separated selector piece
// We only need to split if is actually comma separted(selectors > 1)
if(rule.selectors.length > 1) {
// We only need to split if it's actually a Rule with multiple selectors
MadLittleMods marked this conversation as resolved.
Show resolved Hide resolved
if(rule.type == 'rule' && rule.selectors.length > 1) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use rule.type === 'rule'

Also why are we moving away from walkRules if we then check for rule.type === 'rule' here anyway?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like it is because walkRules doesn't walk over atrule types.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I believed I had responded to this comment. Yes, it's because atrule types were not being traversed by walkRules.

// Reverse the selectors so that we can cloneAfter in the same comma separated order
rulesToWorkOn = rule.selectors.reverse().map(function(selector) {
var ruleClone = rule.cloneAfter();
Expand Down
8 changes: 8 additions & 0 deletions test/fixtures/at-rules-containing-properties.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
:root {
--font-name: 'my-font-family-name';
}

@font-face {
font-family: var(--font-name);
src: url('myfont.woff2') format('woff2');
}
4 changes: 4 additions & 0 deletions test/fixtures/at-rules-containing-properties.expected.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
@font-face {
font-family: 'my-font-family-name';
src: url('myfont.woff2') format('woff2');
}
13 changes: 13 additions & 0 deletions test/fixtures/nested-at-rules-containing-properties.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
:root {
--color: red;
}

/*
Prince XML at-rules.
https://www.princexml.com/doc/11/at-rules/
*/
@page {
@footnote {
background-color: var(--color);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/*
Prince XML at-rules.
https://www.princexml.com/doc/11/at-rules/
*/
@page {
@footnote {
background-color: red;
}
}
3 changes: 3 additions & 0 deletions test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,9 @@ describe('postcss-css-variables', function() {
test('should work with nested @media', 'media-query-nested', { preserveAtRulesOrder: false });
test('should work with nested @media, preserving rule order', 'media-query-nested-preserver-rule-order', { preserveAtRulesOrder: true });

test('should work with at-rules containing properties', 'at-rules-containing-properties');
test('should work with nested at-rules containing properties', 'nested-at-rules-containing-properties');


test('should cascade to nested rules', 'cascade-on-nested-rules');

Expand Down