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

Missing meta.messages property #181

Closed
MikeMcC399 opened this issue May 9, 2024 · 10 comments · Fixed by #182
Closed

Missing meta.messages property #181

MikeMcC399 opened this issue May 9, 2024 · 10 comments · Fixed by #182

Comments

@MikeMcC399
Copy link
Contributor

MikeMcC399 commented May 9, 2024

Suggestion

Support the recommended rule eslint-plugin/prefer-message-ids by providing a meta.messages property using a MessageId. Currently, running yo eslint:rule does not add this property to the generated lib/rules/<rule-id>.js rule.

Background

When reporting a rule violation, it's preferred to provide the violation message with the messageId property instead of the message property.

Documentation

@nzakas
Copy link
Member

nzakas commented May 9, 2024

Thanks for the suggestion. Would you like to submit a PR to add this?

@MikeMcC399
Copy link
Contributor Author

@nzakas

Would you like to submit a PR to add this?

I'm very new to ESLint and at this time I don't feel that I have the skills and experience to implement the suggestion. Sorry!

I just noticed the issue as I was working on resolving a lot of eslint-plugin-cypress upgrade issues which I started on one month ago.

@bmish
Copy link
Sponsor Member

bmish commented May 9, 2024

So you want to add messages: {} with an empty object I assume? I'm open to this if there's interest, since most new rules should be using this property for at least one violation message.

The template is here:

meta: {
type: null, // `problem`, `suggestion`, or `layout`
docs: {
description: "<%- desc.replace(/"/g, '\\"') %>",
recommended: false,
url: null, // URL to the documentation page for this rule
},
fixable: null, // Or `code` or `whitespace`
schema: [], // Add a schema if the rule has options
},

@MikeMcC399
Copy link
Contributor Author

@bmish

So you want to add messages: {} with an empty object I assume?

That would be a minimum and just adding that to the template would be easy to implement.

What I wasn't sure about is how far this should be extended, and if it should be expanded to a dialog which asks for a messageID and a message or assumes some default messageID and adds a dummy message. That's where my lack of experience in this area breaks down!

@bmish
Copy link
Sponsor Member

bmish commented May 9, 2024

I don't think the generator needs to fill in any actual messages. The rule creator probably doesn't have specific messages ready yet when they're just trying to create the barebones rule skeleton.

@MikeMcC399
Copy link
Contributor Author

@bmish

So I suggest to add:

    messages: {} // Add messageId and message

to the template. The linting message will stay the same, however adding the above line into the template means that rule creators should be able to find the place faster, where they need to add a message. No additional information is requested at rule creation time with yo eslint:rule.

Linting error:

error meta.messages must contain at least one violation message eslint-plugin/prefer-message-ids

If that sounds good, then I will submit a PR for this change.

@bmish
Copy link
Sponsor Member

bmish commented May 10, 2024

That sounds reasonable.

@MikeMcC399
Copy link
Contributor Author

@bmish

Should this be a chore (no release), fix (patch release) or feat (minor release)?

It doesn't change any error message, so from that point of view it could be a chore. On the other hand it won't get used unless there is a release.

@bmish
Copy link
Sponsor Member

bmish commented May 10, 2024

feat should be fine. It's not fixing anything, but it is a possible usability improvement.

@MikeMcC399
Copy link
Contributor Author

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

Successfully merging a pull request may close this issue.

3 participants