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

feat(ts): ban const enum #201

Merged
merged 1 commit into from Jun 28, 2023
Merged

feat(ts): ban const enum #201

merged 1 commit into from Jun 28, 2023

Conversation

zanminkian
Copy link
Contributor

Description

There are so many issues about const enum. Just forget this feature and use enum only.
Please check:

Linked Issues

Additional context

@antfu
Copy link
Owner

antfu commented Jun 15, 2023

I am not sure about this:

  • I don't want to add another plugin solely for this rule, also rnx-kit seems to be for React Native
  • This feels a bit over-babysitting to me. Programmers should know what are the limitations of the language etc.

@zanminkian
Copy link
Contributor Author

  • As for the first opinion, maybe I will refactor this PR to implement antfu/no-const-enum rule.
  • As for the second opinion, const enum is the ts grammar which we should not use, like var, with, namespace. It's not over-babysitting. All of them will make our code base hard to maintain. There are no reason to use const enum, except performance reason. const enum have a better performance than enum, but insignificant.

@antfu
Copy link
Owner

antfu commented Jun 15, 2023

Sure, if we move away from the dependency, I am ok to have it. Thanks.

@zanminkian
Copy link
Contributor Author

Done. Please take a review @antfu.

@antfu antfu merged commit 2bf0c8d into antfu:main Jun 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants