Const enum recommendation Eslint plugin

[noahdarveau-MSFT]

For more information about how to contribute to this repo, visit this page.

Description

Summarize the changes, including the goals and reasons for this change. Be sure to call out any technical or behavior changes that reviewers should be aware of.

If this Pull Request should close/resolve any issues when merged, use the special syntax for that here.

Main changes in the PR:

1. I wrote a custom rule plugin for eslint that will give a warning to the developer to consider if the enum they wrote could be const. The plugin will give a warning, but since the repo is configured to treat warnings as errors, they will need to add an eslint ignore line to the line just before their enum if it should not be const. The warning text when eslint detects a non-const enum is Please consider if you can use a const enum for ${enumName} to minimize bundle size. If not, add "/* eslint-disable-next-line recommend-const-enums/recommend-const-enums */" to the line above to disable this warning.

How do people feel about adding this plugin to the repo? I wrote it with the intention of it just being a reminder to people so they stop and think about what they're writing with a little bit more intentionality. On the other hand, I could definitely see it as an unnecessary pain, that could definitely lead to people just blindly following the recommendation without actually thinking about it. So I figured I'd just put this PR out here and see what people thought about it, so some opinions on the matter would be greatly appreciated.
 

Validation

Validation performed:

1. <Step 1>
2. <Step 2>

Unit Tests added:

Unit tests are required for all changes. If no unit tests were added as part of this change, please explain why they aren't necessary.

<Yes/No>

End-to-end tests added:

<Yes/No>

Additional Requirements

Change file added:

Ensure the change file meets the formatting requirements.

<Yes/No>

Related PRs:

Remove this section if n/a

Next/remaining steps:

List the next or remaining steps in implementing the overall feature in subsequent PRs (or is the feature 100% complete after this?).

Remove this section if n/a

• Item 1
• Item 2

Screenshots:

Remove this section if n/a

Before	After
< image1 >	< image2 >

[jadahiya-MSFT]

I personally don't see anything wrong with adding this in. I definitely agree with the sentiment that people will probably just follow the guidance given rather than thinking about it. I think something that could be helpful is if we also were able to show package size increases in the warning.

Just out of curiosity, is there any scenarios that going forward we wouldn't want our enums to be given the const tag?

[noahdarveau-MSFT]

Just out of curiosity, is there any scenarios that going forward we wouldn't want our enums to be given the const tag?

Generally speaking, enums should be const if we're just using them to hold values nice and neatly, but there are some edge cases, like if you wanted to use the reverse mapping functionality or need to access the enum object at runtime, that you would want to keep it a regular enum.

[AE-MS]

I think this should be something we enable in the repo because the default behavior for enums is going to cause the package to grow unnecessarily. Most engineers who add enums will not realize this (myself included). Enabling a rule that causes every engineer who adds an enum to learn about the impact and then make an intentional decision about whether or not it can be const is the right behavior.

Once this rule is enabled, I assume the way that an engineer would "make the decision" is to either add const to their enum or add a rule suppression to the enum line that says "I know what I'm doing, this should really not be const." ? #Closed

[AE-MS]

Noah

lol

[AE-MS]

🕐

[AE-MS]