eslint / archive-website

The ESLint website
https://eslint.org
MIT License
95 stars 244 forks source link

Add warning for suggested changes to rule docs #899

Open btmills opened 2 years ago

btmills commented 2 years ago

In the 2021-12-16 TSC meeting, we discussed feat: Fixer for missing unicode flag in no-misleading-character-class. That PR adds an editor suggestion that will add the u flag. Sometimes, adding that flag may change behavior of other portions of the regular expression unrelated to the misleading character class. While adding the u flag is probably the best fix most of the time, we're implementing the change as a suggestion rather than an autofix because we can't be sure the indirect behavior changes are correct.

More generally, we want to make sure users know that they should consider all possible implications of the behavior change before applying editor suggestions. Currently, documentation pages for rules with suggestions (like no-unsafe-negation) include a light bulb icon and the text "Some problems reported by this rule are manually fixable by editor suggestions."

In the meeting, we agreed to change the wording to "Some problems reported by this rule are manually may be fixable by editor suggestions. (Warning)" and link "Warning" to a page that describes the potential pitfalls.

pranay101 commented 2 years ago

Hi, I am new to open source and I would like to work on this issue. Please guide me further on what needs to be done.

nzakas commented 2 years ago

@pranay101 thanks for the interest! We would need to add a link here: https://github.com/eslint/website/blob/a9e5dbf11025f05d2717a8d855772b55fd070ed9/docs/rules/index.liquid#L10

And then add a page that it links to describing what suggestions are and how they might produce changes that alter the code behavior.

pranay101 commented 2 years ago

hey @nzakas which link would you like me to redirect the users to?

nzakas commented 2 years ago

We can probably start with /user-guide/fixes-and-suggestions. Note that we would also need to add this into the main eslint repo in the docs folder (which is the source of truth for the website)

AkashaRojee commented 2 years ago

Hello!

Is this issue already being worked on, please? If not, then I would like to take it.

Thanks!

nzakas commented 2 years ago

@AkashaRojee you are free to work on it 👍

AkashaRojee commented 2 years ago

@nzakas

Great!

I worked on a first draft of the fixes and suggestions for no-unsafe-negation.

Please, find it here.

I have a few questions about the next steps. Once they are confirmed, I will proceed to the fixes and suggestions for the other rules.

1. Is the content in the first draft correct?

I used the information from the rule's test source.

Is there anything I should add/change?

2. Which folder/file structure is preferred?

Option 1: /user-guide/fixes-and-suggestions.md, with a section for each rule

Option 2: /user-guide/fixes-and-suggestions/rule-name.md, for each rule (and an index.liquid page under /fixes-and-suggestions)

Depending on the above, I will then add the correct fixes-and-suggestions link to the following, as you mentionned above.

https://github.com/eslint/website/blob/a9e5dbf11025f05d2717a8d855772b55fd070ed9/docs/rules/index.liquid#L10

3. How to proceed with the PRs for the website repo and the main repo?

I understand that we would also need to add this content into the main repo in /docs which is the source of the truth for the website. Do I create a PR for the file both in the website repo and the main repo?

While inspecting the main repo, I saw that a Makefile is used to automate the generation of parts of the documentation, e.g.:

Should I remove these sections from the file, and modify the Makefile accordingly?

Thanks!

nzakas commented 2 years ago

Thanks so much for working on this. I think we may have a misunderstanding here: we aren’t looking for individual updates to rules where we list what happens with each suggestion. All we are doing here is adding a link from the rules index page that explains suggestions in all rules may not be safe.

As such, option 1 is the best choice to display a generic message about why suggestions may not be safe.

Regarding where to make changes: the content of the warning should be in the eslint repo. There is no need to make changes to the Makefile. You can then add a link to the rules index in the website repo. We will wait to merge that until the next release, when the new content from the eslint repo is copied over.

AkashaRojee commented 2 years ago

@nzakas Thanks for the clarification. Sorry for the delay, I was out of action. I'll get to it this week 🚀

nzakas commented 2 years ago

No problem, thanks for the followup.