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

stylelint message not showing interpolated values #142

Closed
anoopsinghbayes opened this issue Feb 24, 2021 · 16 comments · Fixed by #143
Closed

stylelint message not showing interpolated values #142

anoopsinghbayes opened this issue Feb 24, 2021 · 16 comments · Fixed by #143
Assignees

Comments

@anoopsinghbayes
Copy link

anoopsinghbayes commented Feb 24, 2021

I am struggling with message

module.exports = {
  plugins: ["stylelint-declaration-strict-value"],
  rules: {
    "scale-unlimited/declaration-strict-value": [
      ["/color$/", "fill", "stroke"], {
        ignoreVariables:false,
        message: "Custom expected ${types} for \"${value}\" of \"${property}\"",
      }],
  },
};

and this what i see in vscode
image

should'nt it show the interpolated values here
or am i missing some thing

@AndyOGo
Copy link
Owner

AndyOGo commented Feb 24, 2021

Oh, that looks not as it should be.

I have a test here:
https://github.com/AndyOGo/stylelint-declaration-strict-value/blob/master/test/custom-message.js

may I ask you to give more information about version of stylelint, this plugin, node?

@anoopsinghbayes
Copy link
Author

"stylelint": "^13.11.0"
"stylelint-declaration-strict-value": "^1.7.7"
node :13.7.0

and did try this its the same issue

message: 'Custom expected ${types} for "${value}" of "${property}"', // eslint-disable-line no-template-curly-in-string

@AndyOGo
Copy link
Owner

AndyOGo commented Feb 24, 2021

Thank you.
I need to look into it.

@AndyOGo
Copy link
Owner

AndyOGo commented Feb 24, 2021

@anoopsinghbayes
I added your case to my tests, but it succesfully runs 🤔
https://github.com/AndyOGo/stylelint-declaration-strict-value/pull/143/files

Is there any other plugin you are using, or a VS code extension?
Does it work if you don't use the message option?

@AndyOGo AndyOGo added help wanted and removed bug labels Feb 24, 2021
@anoopsinghbayes
Copy link
Author

it does work without the message option
both in vscode and from the terminal
image

image

@AndyOGo
Copy link
Owner

AndyOGo commented Feb 25, 2021

Great.

So I assume with the message option its broken in both, VScode extension and the terminal?

It would be very helpful if I could have a look myself and see what you are seeing.
Is your code public?

Btw. this code does the interpolation.
https://github.com/AndyOGo/stylelint-declaration-strict-value/blob/7f9475d9f701ac9a1657f33b1f59aa184ab9727a/src/lib/validation.ts#L218,L221

@anoopsinghbayes
Copy link
Author

I don't have the repo public
But I can put something together real quick tomorrow and will let you know

@AndyOGo
Copy link
Owner

AndyOGo commented Feb 25, 2021

That would be awesome, thank you

@AndyOGo
Copy link
Owner

AndyOGo commented Feb 25, 2021

I doublechecked the stylelint documentation.
Hope message is not a reserved secondary option.
https://stylelint.io/user-guide/configure#message

stylelint/stylelint#836

This PR added the message option
https://github.com/stylelint/stylelint/pull/672/files

@AndyOGo
Copy link
Owner

AndyOGo commented Feb 25, 2021

@anoopsinghbayes Thanks for the issue.

I released version 1.7.8 which should fix this by undoing stylelint's internal custom message handling.
Hope it works that way, unfortunately stylint's test utils don't seem to consider stylelint itself, so I couldn't add a unit test for it and it may break in future versions of stylelint.

For details either have a look into the PR #143 or I also commented the official feature request here

@AndyOGo
Copy link
Owner

AndyOGo commented Feb 25, 2021

I created a follow up issue stylelint/stylelint#5170

And a related issue exists for general message substitution support
stylelint/stylelint#4117

@anoopsinghbayes
Copy link
Author

anoopsinghbayes commented Feb 25, 2021

@AndyOGo your bug fix did the trick ,thanks a lot
i had a follow up question
can i have separate message for different selector
for eg
/color$/ ==>message "use a color function xyz"
"padding==>message "use padding values abc"

I see something for ignoreValues which takes an object to do this split ,but nothing for message
// .stylelintrc "rules": { // ... "scale-unlimited/declaration-strict-value": [ ["/color$/", "fill", "stroke"], { ignoreValues: { "/color$/": ["currentColor", "/^#[0-9a-fA-F]{3,6}$/", "inherit"], "fill": ["currentColor", "inherit"], "stroke": "currentColor", "z-index": "/^\\d+$/", }, }], // ... }

i did look in the scheme section ,didn't find anything that could help am i missing something

@AndyOGo
Copy link
Owner

AndyOGo commented Feb 25, 2021

Thanks for your reply.
Glad to hear it works :)

Keep in mind though that it's a hacky workaround, wich messes with stylelint's internals.
This should become an official API as part of stylelint stylelint/stylelint#5170


No, that's not possible as of now.
The only options which supports that is ignoreValues, because it needs it.

Why would you need that?

@anoopsinghbayes
Copy link
Author

anoopsinghbayes commented Feb 25, 2021

we have a design system with a bunch of utility classes
like abc-padding-level1 abc-padding-level2 ,abc-margin-level1 etc for almost everything
we want to stop devs from using fixed value in the css either use the utility class or a scss function in place of fixed value

for eg if some one does this

`.custom-padding-class{
padding:6px; ///show stylelint error message "use the abc-padding-level1 class in the html or use padding-
//function(level1)"
color:red; // show stylelint error message "use the abc-color-error class in the html or use color-function("error")

}`
something like this

i can take care of the fixes in the autofixer but the message would be great for teaching best practices for our team

@AndyOGo
Copy link
Owner

AndyOGo commented Feb 25, 2021

Thanks, now I understand why you want to have that specific messages.

@AndyOGo
Copy link
Owner

AndyOGo commented Feb 25, 2021

Alright, I create a follow-up issue to elaborate the idea #146
Not that it will be implemented soon.

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

Successfully merging a pull request may close this issue.

2 participants