-
-
Notifications
You must be signed in to change notification settings - Fork 938
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
Add new shorthand data #2354
Add new shorthand data #2354
Conversation
Why is
I agree, but I don't like that we're removing functionality for that. Can we use Autoprefixer in the rule to remove the need for a manual list?
Sounds good to me! Want to add the changes above to this PR? |
I know it's real, but I don't know every property for this shorthand. MDN and W3C don't make it clear. They say about
I will look how
Sure! |
@hudochenkov This page looks helpful: https://tympanus.net/codrops/css_reference/mask/ |
Benchmarks for ac40d7a Before:
After:
|
…lock-no-shorthand-property-overrides
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hudochenkov Nice work! Left a few comments.
- `grid-template` | ||
- `outline` | ||
- `text-decoration` | ||
- `text-emphasis` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, this is a lot of new power!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is looking great!
} | ||
|
||
check(node) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this change relevant?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's kind of irrelevant. Benchmarks here. Should I revert this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this change the only thing that caused the slight speed-up? Does such a change reliably cause speed-ups in other rules? What we had before matches a pattern that is widely used throughout the rules, so I'd prefer to stick with it unless we get some significant benefit from changing it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reverted.
@@ -15,7 +14,14 @@ const messages = ruleMessages(ruleName, { | |||
rejected: (unexpected, expected) => `Unexpected longhand value '${unexpected}' instead of '${expected}'`, | |||
}) | |||
|
|||
const shorthandableProperties = new Set(Object.keys(shorthandData)) | |||
const shorthandableProperties = new Set([ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please add a comment explaining why these specific properties are checked, and not others?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Not sure if text correct, though. Also added grid-gap
. Maybe some other Grid properties can be added, but I can't say for sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just left the one comment about a potentially irrelevant change. Once we address that, I think this is ready to merge, right?
} | ||
|
||
check(node) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this change the only thing that caused the slight speed-up? Does such a change reliably cause speed-ups in other rules? What we had before matches a pattern that is widely used throughout the rules, so I'd prefer to stick with it unless we get some significant benefit from changing it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks!
Added to changelog:
|
* Add stylelint semantic versioning policy * Fix whitespace and empty lines * Add changelog entry * Remove stylelint-config=standard reference * Tweak major release optional options verbiage * Update changelog * Props ESLint * Fix false negatives with ignore: ["descendant"] in selector-no-type (#2200) * Update CHANGELOG.md * Fix handling of shared-line comments (#2262) * Fix handling of shared-line comments Introduces a number of utilities and adjusts the following rules: at-rule-empty-line-before, custom-property-empty-line-before, declaration-empty-line-before, rule-nested-empty-line-before, rule-non-nested-empty-line-before Closes #2237; closes #2239; closes #2240. * Fixes based on feedback * Use getPreviousNonSharedLineCommentNode more * Use more new helpers in comment-empty-line-before * Add ignore: ["child"] option to selector-no-type (#2217) * Add ignore: ["child"] option to selector-no-type * Correct logic for ignore: ["child"] option * Include line and column in reject tests with multiple type selectors * Update CHANGELOG.md * Report every selector resolved selector in selector-max-compound-selectors (#2350) * Update CHANGELOG.md * Add new shorthand data (#2354) * Add new shorthand data * Add mask shorthand * Use defined list of properties in shorthand-property-no-redundant-values * Improve declaration-block-no-shorthand-property-overrides speed a little bit * Check prefixed properties agains prefixed properties in declaration-block-no-shorthand-property-overrides * Improve declaration-block-no-redundant-longhand-properties speed a little bit * Ignore prefixes case in declaration-block-no-shorthand-property-overrides * Check prefixed properties agains prefixed properties in declaration-block-no-redundant-longhand-properties * Add `grid-gap` to shorthand-property-no-redundant-values * Explain properties set for shorthand-property-no-redundant-values * Revert unnecessary changes * Add “mask” to declaration-block-no-redundant-longhand-properties readme * Update CHANGELOG.md * Update changelog * Remove deprecated rules (#2422) * Remove deprecated rules * Include `rule-empty-line-before` in system tests * *-empty-line-before rules consider line as empty if it contains whitespace only (#2440) * Update CHANGELOG.md * Remove deprecated options (#2433) * Update CHANGELOG.md * report-needless-disables now exits with non-zero code (#2341) * Update CHANGELOG.md * Remove leftover files (#2442) * Remove leftover file * One more * And one more * Ignoring improvements (#2464) * WIP ignoring improvements * Remove standaloneIgnored property of result * Add disableDefaultIgnores * Add documentation tweaks * Add parseStylelintIgnore tests * Non-absolutized ignore file paths * Restire gitignore syntax * Remove parseStylelintIgnore * Update CHANGELOG.md * Check all linear-gradients in a value list (#2496) Closes #2481 * Update CHANGELOG.md * Update CHANGELOG.md * Respect line case for `ignorePattern` (#2683) Closes #2647 * Update CHANGELOG.md * Update decls * Fix remark warning * Update snapshots * Add missing message to reject test * Fix tests for *-empty-line-before rules (#2688) * Fix tests for *-empty-line-before rules * Test addition and tweak * Update to PostCSS@6 (#2561) * Update to PostCSS@6 * Fix PostCSS@6 closing-brace test (#2689) * Account for PostCSS@6's support for at-apply * Remove only * Fix postcssPlugin tests (#2690) * Fix defaultSeverity.test.js test * Fix ignoreDisables test * Check ownSemiColon * Fixed: remove hacks related to custom property set. * Remove deprecated rules (#2693) Closes #2521 * Update CHANGELOG.md * Update CHANGELOG.md * Add VISION.md (#2704) * Add releases.md (#2705) * Add releases.md Closes #2700 * Add dry-release step * Use HTTPS for links * Change ignore options for selector-max-type (#2701) * Fix false negatives with ignore descendant * Add ignore child Closes #2699 * Update CHANGELOG.md * Update README.md to account for VISION.md (#2727) * Update README.md to account for VISION.md * Fix lint warnings in CHANGELOG * Update postcss-less to 1.0 (#2702) * Update postcss-less to 1.0.2 * Update isStandardSyntaxRule for postcss-less 1.0 * Pass custom postcss stringifier when creating result * Correct no-extra-semicolons Less test position * Ignore Less mixins in no-extra-semicolons * Update postcss-less to 1.1.0 * Correct no-extra-semicolons Less test position again * Utilize isStandardSyntaxRule when checking for Less rules in no-extra-semicolons * Case sensitive white/blacklists and ignore* options (#2709) Closes #2660 * Update CHANGELOG.md * docs: Add "New Release Issue Template" to releases.md (#2728) * Exit with non-zero code on postcss warnings (#2713) * Enhancement: exit with non zero code on postcss warnings. * Account for parseErrors in stringformatqer * Guard against no parseErrors * Update CHANGELOG.md * Refactor: simplify ignore logic. (#2732) * Update changelog * Bump 8.0.0 changes to top * Correctly place changelog items * Add introduction * Add codefence for website * Use relative path * Include VISION in package for website * Add link to VISION doc for website * Changelog amends * Update releases.md (#2738) * Update eslint and config (#2737) * Order requires * Use eqeqeq eslint rule * Update eslint * Disable no-useless-escape just for this repo * Enable no-useless-escape rule * Use tilde * Fix enable comment * Add reject test for unescaped regex
#1780
Added new shorthand properties from #1780, except
mask
, because it's not clear what for this shorthand.Removed prefixed shorthands. I don't think it's a good idea to keep list of all prefixed properties and shorthand.
I have concerns about
shorthand-property-no-redundant-values
. It usesshorthandData
also. My concers is that it has a list of ignored shorthands. With new shorthands ignore list will be much bigger than list of shorthands supported in this rule. Maybe we should specify the list of allowed shorthands, rather usingshorthandData
and ignore.