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

fix(scripts): Use the SCSS shared stylelint-config-wordpress config #17060

Merged
merged 2 commits into from
Aug 26, 2019

Conversation

ntwb
Copy link
Member

@ntwb ntwb commented Aug 16, 2019

Description

When using the lint-style wp-scripts command the SCSS rules from stylelint-config-wordpress are not used, this results in lint errors being displayed when linting SCSS files.

"lint:css": "wp-scripts lint-style",

By updating the config to use the stylelint-config-wordpress/scss shared config then both CSS and SCSS files are linted correctly.

How has this been tested?

Download a valid SCSS file, e.g. scss-valid.scss and run the lint-style script against the scss-valid.scss file:

./node_modules/.bin/wp-scripts lint-style 'scss-valid.scss'

If successful there should be no errors displayed.

If unsuccessful the following errors will be displayed:

scss-valid.scss
  3:1  ✖  Unexpected unknown at-rule "@function"   at-rule-no-unknown               
  5:2  ✖  Unexpected unknown at-rule "@return"     at-rule-no-unknown               
  8:1  ✖  Unexpected unknown at-rule "@mixin"      at-rule-no-unknown               
 19:1  ✖  Unexpected unknown at-rule "@if"         at-rule-no-unknown               
 21:2  ✖  Expected newline after "}"               block-closing-brace-newline-after
 21:3  ✖  Expected empty line before at-rule       at-rule-empty-line-before        
 21:3  ✖  Unexpected unknown at-rule "@else"       at-rule-no-unknown               
 30:2  ✖  Unexpected unknown at-rule "@include"    at-rule-no-unknown               
 31:2  ✖  Unexpected unknown at-rule "@include"    at-rule-no-unknown

Screenshots

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

@ntwb ntwb added [Type] Bug An existing feature does not function as intended [Package] Scripts /packages/scripts labels Aug 16, 2019
Copy link
Member

@jorgefilipecosta jorgefilipecosta left a comment

Choose a reason for hiding this comment

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

Hi @ntwb thank you for looking into this problem,
When I execute ./node_modules/.bin/wp-scripts lint-style 'scss-valid.scss'
I get the following error:

 21:2  ✖  Expected newline after "}"   block-closing-brace-newline-after

I get exactly the same error in master. Is that expected? I'm doing something wrong?

@ntwb
Copy link
Member Author

ntwb commented Aug 19, 2019

@jorgefilipecosta That's correct, its also the current behaviour which this PR is aiming to fix.

The reason you're getting this error still is because the old stylelint config is still being used, you'd need to remove the node_modules from both the repo root and /packages/wp-scripts and then npm install after checking out this branch in both of those same folders.

@gziolo
Copy link
Member

gziolo commented Aug 26, 2019

Shouldn't we apply it to the config file in the root of the Gutenberg project? I mirrored it in @wordpress/scripts and that's why it looks like it is now.

@gziolo
Copy link
Member

gziolo commented Aug 26, 2019

Hi @ntwb thank you for looking into this problem,
When I execute ./node_modules/.bin/wp-scripts lint-style 'scss-valid.scss'
I get the following error:

 21:2  ✖  Expected newline after "}"   block-closing-brace-newline-after

I get exactly the same error in master. Is that expected? I'm doing something wrong?

I see the same issue unless I update

"extends": "stylelint-config-wordpress",

to use scss config as well. However, it uncovers a few lint violations for the existing SCSS files:

> wp-scripts lint-style '**/*.scss'


assets/stylesheets/_mixins.scss
 367:3  ✖  Unexpected newline after "}" of @if statement        scss/at-if-closing-brace-newline-after  
 367:3  ✖  Expected single space after "}" of @if statement     scss/at-if-closing-brace-space-after    
 369:2  ✖  Unxpected empty line before @else                    scss/at-else-empty-line-before          
 373:3  ✖  Unexpected newline after "}" of @else statement      scss/at-else-closing-brace-newline-after
 373:3  ✖  Expected single space after "}" of @else statement   scss/at-else-closing-brace-space-after  
 375:2  ✖  Unxpected empty line before @else                    scss/at-else-empty-line-before          

packages/block-library/src/classic/editor.scss
 117:3  ✖  Unnecessary nesting selector (&)   scss/selector-no-redundant-nesting-selector

packages/block-library/src/code/editor.scss
 42:2  ✖  Unnecessary nesting selector (&)   scss/selector-no-redundant-nesting-selector

packages/block-library/src/gallery/style.scss
 82:2  ✖  Unnecessary nesting selector (&)   scss/selector-no-redundant-nesting-selector
 83:2  ✖  Unnecessary nesting selector (&)   scss/selector-no-redundant-nesting-selector

packages/block-library/src/navigation-menu-item/editor.scss
 65:2  ✖  Unnecessary nesting selector (&)   scss/selector-no-redundant-nesting-selector
 69:2  ✖  Unnecessary nesting selector (&)   scss/selector-no-redundant-nesting-selector

packages/block-library/src/pullquote/editor.scss
  4:3  ✖  Unnecessary nesting selector (&)   scss/selector-no-redundant-nesting-selector
 11:2  ✖  Unnecessary nesting selector (&)   scss/selector-no-redundant-nesting-selector
 21:2  ✖  Unnecessary nesting selector (&)   scss/selector-no-redundant-nesting-selector

packages/components/src/button-group/style.scss
 8:3  ✖  Unnecessary nesting selector (&)   scss/selector-no-redundant-nesting-selector

packages/components/src/color-palette/style.scss
 30:2  ✖  Unnecessary nesting selector (&)   scss/selector-no-redundant-nesting-selector
 54:3  ✖  Unnecessary nesting selector (&)   scss/selector-no-redundant-nesting-selector

packages/components/src/toolbar/style.scss
 10:2  ✖  Unnecessary nesting selector (&)   scss/selector-no-redundant-nesting-selector
 20:2  ✖  Unnecessary nesting selector (&)   scss/selector-no-redundant-nesting-selector

packages/components/src/toolbar-button/style.scss
 27:2  ✖  Unnecessary nesting selector (&)   scss/selector-no-redundant-nesting-selector

packages/block-editor/src/components/block-navigation/style.scss
 47:2  ✖  Unnecessary nesting selector (&)   scss/selector-no-redundant-nesting-selector

packages/edit-post/src/components/visual-editor/style.scss
 5:2  ✖  Unnecessary nesting selector (&)   scss/selector-no-redundant-nesting-selector

Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

We can land it as is, but we will need a follow-up for the Gutenberg itself.

@gziolo gziolo merged commit aeac9bd into master Aug 26, 2019
@gziolo gziolo deleted the fix/scripts-stylelint-config branch August 26, 2019 12:46
@gziolo gziolo added this to the Gutenberg 6.4 milestone Aug 26, 2019
donmhico pushed a commit to donmhico/gutenberg that referenced this pull request Aug 27, 2019
…ordPress#17060)

* fix(scripts): use the SCSS shared stylelint-config-wordpress config

* Update CHANGELOG.md
gziolo pushed a commit that referenced this pull request Aug 29, 2019
…17060)

* fix(scripts): use the SCSS shared stylelint-config-wordpress config

* Update CHANGELOG.md
gziolo pushed a commit that referenced this pull request Aug 29, 2019
…17060)

* fix(scripts): use the SCSS shared stylelint-config-wordpress config

* Update CHANGELOG.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Scripts /packages/scripts [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants