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.

Sign up for GitHub

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

Jump to bottom

Fix isStandardSyntaxAtRule.test.js that use callbacks #4957

Merged
merged 2 commits into from
Oct 4, 2020
Merged

Fix isStandardSyntaxAtRule.test.js that use callbacks #4957

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0cb13a3
Fix `isStandardSyntaxAtRule.test.js` that use callbacks
ybiquitous Sep 25, 2020
c88a46d
Add `atRule()` helper to improve readability
ybiquitous Sep 27, 2020
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
124 changes: 50 additions & 74 deletions lib/utils/__tests__/isStandardSyntaxAtRule.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,136 +1,112 @@
'use strict';

const isStandardSyntaxAtRule = require('../isStandardSyntaxAtRule');
const less = require('postcss-less');
const postcss = require('postcss');
const sass = require('postcss-sass');
const scss = require('postcss-scss');
const postcssLess = require('postcss-less');
const postcssSass = require('postcss-sass');
const postcssScss = require('postcss-scss');
Copy link
Member Author

Choose a reason for hiding this comment

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

[note] I've renamed the variables not to hide local variables with the same name, e.g.

const sass = '@mixin mixin()\n  @content';
const scss = '@mixin mixin() { @content; };';


describe('isStandardSyntaxAtRule', () => {
it('non nested at-rules without quotes', () => {
atRules('@charset UTF-8;', (atRule) => {
expect(isStandardSyntaxAtRule(atRule)).toBeTruthy();
});
expect(isStandardSyntaxAtRule(atRules('@charset UTF-8;')[0])).toBeTruthy();
ybiquitous marked this conversation as resolved.
Show resolved Hide resolved
});

it("non nested at-rules with `'` quotes", () => {
atRules("@charset 'UTF-8';", (atRule) => {
expect(isStandardSyntaxAtRule(atRule)).toBeTruthy();
});
expect(isStandardSyntaxAtRule(atRules("@charset 'UTF-8';")[0])).toBeTruthy();
});

it('non nested at-rules with `"` quotes', () => {
atRules('@charset "UTF-8";', (atRule) => {
expect(isStandardSyntaxAtRule(atRule)).toBeTruthy();
});
expect(isStandardSyntaxAtRule(atRules('@charset "UTF-8";')[0])).toBeTruthy();
});

it("non nested at-rules with `'` quotes and without space after name", () => {
atRules("@charset'UTF-8';", (atRule) => {
expect(isStandardSyntaxAtRule(atRule)).toBeTruthy();
});
expect(isStandardSyntaxAtRule(atRules("@charset'UTF-8';")[0])).toBeTruthy();
});

it('non nested at-rules with `"` quotes and without space after name', () => {
atRules('@charset"UTF-8";', (atRule) => {
expect(isStandardSyntaxAtRule(atRule)).toBeTruthy();
});
expect(isStandardSyntaxAtRule(atRules('@charset"UTF-8";')[0])).toBeTruthy();
});

it('non nested at-rules with function and without space after name', () => {
atRules('@import url("fineprint.css") print;', (atRule) => {
expect(isStandardSyntaxAtRule(atRule)).toBeTruthy();
});
expect(isStandardSyntaxAtRule(atRules('@import url("fineprint.css") print;')[0])).toBeTruthy();
});

it('nested at-rules', () => {
atRules('@media (min-width: 100px) {};', (atRule) => {
expect(isStandardSyntaxAtRule(atRule)).toBeTruthy();
});
expect(isStandardSyntaxAtRule(atRules('@media (min-width: 100px) {};')[0])).toBeTruthy();
});

it('nested at-rules with newline after name', () => {
atRules('@media\n(min-width: 100px) {};', (atRule) => {
expect(isStandardSyntaxAtRule(atRule)).toBeTruthy();
});
expect(isStandardSyntaxAtRule(atRules('@media\n(min-width: 100px) {};')[0])).toBeTruthy();
});

it('nested at-rules with windows newline after name', () => {
atRules('@media\r\n(min-width: 100px) {};', (atRule) => {
expect(isStandardSyntaxAtRule(atRule)).toBeTruthy();
});
expect(isStandardSyntaxAtRule(atRules('@media\r\n(min-width: 100px) {};')[0])).toBeTruthy();
});

it('nested at-rules without space after name', () => {
atRules('@media(min-width: 100px) {};', (atRule) => {
expect(isStandardSyntaxAtRule(atRule)).toBeTruthy();
});
expect(isStandardSyntaxAtRule(atRules('@media(min-width: 100px) {};')[0])).toBeTruthy();
});

it('ignore `@content` inside mixins newline', () => {
const sass = '@mixin mixin()\n @content';
// eslint-disable-next-line jest/no-disabled-tests -- TODO: `postcss-sass` parser does not support `@mixin`.
it.skip('ignore `@content` inside mixins newline', () => {
Copy link
Member Author

Choose a reason for hiding this comment

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

[note] This skip does not affect on the coverage:

image

Copy link
Member Author

Choose a reason for hiding this comment

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

See also https://coveralls.io/builds/33727095/source?filename=lib/utils/isStandardSyntaxAtRule.js

Copy link
Member

Choose a reason for hiding this comment

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

Has anybody left an issue on postcss-sass about this? Maybe we can submit a fix upstream as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

I found a PR to add the support of @mixin at-rule, but it seems WIP (and too old). 😢

AleshaOleg/postcss-sass#56

const rules = sassAtRules('@mixin mixin()\n @content');

sassAtRules(sass, (atRule) => {
if (atRule.name === 'mixin') {
return;
}

expect(isStandardSyntaxAtRule(atRule)).toBeFalsy();
});
expect(rules).toHaveLength(2);
expect(rules.map((rule) => rule.name)).toEqual(['mixin', 'content']);
expect(isStandardSyntaxAtRule(rules[0])).toBeTruthy();
expect(isStandardSyntaxAtRule(rules[1])).toBeFalsy();
});

it('ignore `@content` inside mixins space', () => {
const scss = '@mixin mixin() { @content; };';

scssAtRules(scss, (atRule) => {
if (atRule.name === 'mixin') {
return;
}
const rules = scssAtRules('@mixin mixin() { @content; };');

expect(isStandardSyntaxAtRule(atRule)).toBeFalsy();
});
expect(rules).toHaveLength(2);
expect(rules.map((rule) => rule.name)).toEqual(['mixin', 'content']);
expect(isStandardSyntaxAtRule(rules[0])).toBeTruthy();
expect(isStandardSyntaxAtRule(rules[1])).toBeFalsy();
});

it('ignore passing rulesets to mixins', () => {
const less = '@detached-ruleset: { background: red; }; .top { @detached-ruleset(); }';
const rules = lessAtRules(
'@detached-ruleset: { background: red; }; .top { @detached-ruleset(); }',
);

lessAtRules(less, (atRule) => {
expect(isStandardSyntaxAtRule(atRule)).toBeFalsy();
});
expect(rules).toHaveLength(2);
expect(isStandardSyntaxAtRule(rules[0])).toBeFalsy();
expect(isStandardSyntaxAtRule(rules[1])).toBeFalsy();
});

it('ignore calling of mixins', () => {
const less = 'a { .mixin(); }';
const rules = lessAtRules('a { .mixin(); }');

lessAtRules(less, (atRule) => {
expect(isStandardSyntaxAtRule(atRule)).toBeFalsy();
});
expect(rules).toHaveLength(1);
expect(isStandardSyntaxAtRule(rules[0])).toBeFalsy();
});

it('ignore variables', () => {
const less = `
@my-variable: 10px;
.top { margin-top: @my-variable; }
`;

lessAtRules(less, (atRule) => {
expect(isStandardSyntaxAtRule(atRule)).toBeFalsy();
});
const rules = lessAtRules('@my-variable: 10px; .top { margin-top: @my-variable; }');
Copy link
Member Author

Choose a reason for hiding this comment

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

[note] I think the one-line is easier to read here such as '@detached-ruleset: { background: red; }; .top { @detached-ruleset(); }'.


expect(rules).toHaveLength(1);
expect(isStandardSyntaxAtRule(rules[0])).toBeFalsy();
});
});

function atRules(css, cb) {
postcss.parse(css).walkAtRules(cb);
function atRules(code, parser = postcss) {
const rules = [];

parser.parse(code).walkAtRules((rule) => rules.push(rule));

return rules;
}

function sassAtRules(css, cb) {
sass.parse(css).walkAtRules(cb);
function sassAtRules(code) {
return atRules(code, postcssSass);
}

function scssAtRules(css, cb) {
scss.parse(css).walkAtRules(cb);
function scssAtRules(code) {
return atRules(code, postcssScss);
}

function lessAtRules(css, cb) {
less.parse(css).walkAtRules(cb);
function lessAtRules(code) {
return atRules(code, postcssLess);
}