Fix false positives for variables in selector-type-case #3395
Conversation
|
I don't know why tests not passed Now using node v6.14.3 (npm v3.10.10)
iMac--USER:stylelint yozhikm$ yarn test system-tests/fix/fix.test.js
yarn run v1.7.0
$ npm-run-all --serial lint flow
$ npm-run-all --parallel lint:*
$ eslint . --cache
$ remark . --quiet --frail
$ flow
No errors!
$ jest --coverage system-tests/fix/fix.test.js
PASS system-tests/fix/fix.test.js
fix
✓ expected stylelint results (162ms)
✓ expected fixes to PostCSS Result (34ms)
✓ overwrites the original file (38ms)
✓ doesn't write to ignored file (8ms)
✓ outputs fixed code when input is code string (23ms) |
I've restarted it. |
|
@YozhikM thank you for adding a test. Can you add a test for every new condition you've added to that utility, please? Currently, it covers only one case. |
| normalizedValue = normalizedValue.slice(1); | ||
| } | ||
|
|
||
| // SCSS variable |
There was a problem hiding this comment.
I think we need to look into this issue more closely.
I think we need to:
- establish how we test against a non-standard custom syntax i.e.
postcss-jsxin this instance - confirm exactly what syntax is causing the original bug
The first one could involve a change to testRule so that the syntax property accepts a parser function as well as a string. We'd then include postcss-jsx as a dev dependency.
The latter one needs an investigation into what non-standard syntax can be used in a selector. For example, I'm pretty sure that within Scss and Less you can only use variables within interpolation, which makes those conditionals in the isStandard* util redundant.
Additionally, it might be worth waiting until #3284 is merged before revisiting this issue as version 4 of the selector parser is less likley to categorize these things as tag selectors.
There was a problem hiding this comment.
In which cases is the percent sign used before the selector?
There was a problem hiding this comment.
@jeddy3 I agree with your opinion. Shall we put it off for the present?
There was a problem hiding this comment.
In which cases is the percent sign used before the selector?
I believe the Scss placeholder.
Shall we put it off for the present?
I think so.
|
Assigned label “blocked”, as we agreed to wait for #3284 to be merged first. |
jeddy3
changed the title
|
The other issue has been merged so it appears this can now be unblocked. |
|
Yes, this PR can be continue |
|
Blocked by #3988 |
|
@YozhikM looks like PR we though is blocking this PR is not going to be merged soon. Could you resolve merge conflicts, so we can review again, please? |
|
Resolved by addition of |
Closes #3391
No.