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

declaration-block-trailing-semicolon triggers for style property with JSX syntax #3828

Closed
SebastiaanNijland opened this issue Nov 28, 2018 · 6 comments

Comments

@SebastiaanNijland
Copy link

SebastiaanNijland commented Nov 28, 2018

Clearly describe the bug

The declaration-block-trailing-semicolon (set to "always") triggers when I have the following code in a JSX file:
style={{ width: '1rem' }}
Even if I add a semicolon like so:
style={{ width: '1rem;' }}
it still triggers and wants me to keep adding semicolons.

Which rule, if any, is the bug related to?

declaration-block-trailing-semicolon

What CSS is needed to reproduce the bug?

Use CSS in some JSX file:

<div style={{ width: '1rem' }}>

What stylelint configuration is needed to reproduce the bug?

{
  "rules": {
    "declaration-block-trailing-semicolon": "always"
  }
}

Which version of stylelint are you using?

9.9.0 but this bug has been around for a while

How are you running stylelint: CLI, PostCSS plugin, Node API?

This triggers in my IDE (visual code). When running the lintert through the CLI, it only checks our scss files.

Does the bug relate to non-standard syntax (e.g. SCSS, Less etc.)?

Yes, it seems related to checking css syntax in jsx files.

What did you expect to happen?

No error to be flagged or no linting to happen at all (or, to have this property excluded when parsing non-css files).

What actually happened (e.g. what warnings or errors did you get)?

IDE (visual code) shows: Expected a trailing semicolon

@jeddy3
Copy link
Member

jeddy3 commented Nov 28, 2018

@SebastiaanNijland Thanks for the report and for using the template.

I think this is more to do with how the Visual Code extension works than stylelint itself. I believe the extension checks, by default, all of the file types that stylelint supports e.g. .css, .scss, .md, .js etc.

So, it's a case of opting-out of the file types you don't want to check when using the extension. I believe this opt-out behaviour is normal for editor extensions. Unfortunately, this is the opposite of the current stylelint CLI behaviour where you opt-in to file types using a glob e.g. "**/*.{css, scss}".

Fortunately, I believe the extension respects the ignore options in stylelint. So, you can either use the ignoreFiles configuration object property or the .stylelintignore file to get the extension to ignore .js files e.g.

// .stylelintrc
{
  "ignoreFiles": ["**/*.js"],
  "rules": { .. }
}
// .stylelintignore
**/*js

Going forward, there's an issue (#3411) open to allow stylelint to take an opt-out approach on the CLI. I think this will make it easier for users as it'll allow them to align the CLI and editor extension behaviours.

to have this property excluded when parsing non-css files

There's a issue (#3128) to handle this in a sustainable way.

Both issues are labelled as "help wanted" and are looking for someone to champion them. I think they're good issues because when combined together they'll improve the experience for users who lint styles across different file types.

@SebastiaanNijland
Copy link
Author

SebastiaanNijland commented Nov 28, 2018

Thanks for the response! Fiddling with the ignore options seems to solve the issue for my use case.

@jeddy3 jeddy3 closed this as completed Nov 28, 2018
@dreyks
Copy link
Contributor

dreyks commented Nov 26, 2019

what if I do want to lint js files - I'm using css-in-js? I do want a declaration-block-trailing-semicolon everywhere except for the inline style={} attributes

@dreyks
Copy link
Contributor

dreyks commented Nov 26, 2019

I've patched stylelint locally, added

if (decl.parent.type === 'object') {
  return;
}

to

root.walkDecls((decl) => {
if (decl !== decl.parent.last) {
return;
}
checkLastNode(decl);
});

Works for my particular case but has to be generalized for public usage. ping me if you're interested in this discussion

@leodr
Copy link

leodr commented May 11, 2020

Why did this issue get closed? The above comment is still relevant.

@jeddy3
Copy link
Member

jeddy3 commented May 11, 2020

Why did this issue get closed?

Dupe of #4650, which has a pending pull request (#4749) and should hopefully be fixed in the next release.

Also related is #4752, which wants to solve these punctuation issues for all rules in a consistent way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants