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

Replace enums with string literals #190

Merged
merged 1 commit into from
Jul 30, 2023

Conversation

bjornstar
Copy link
Contributor

Replaced the enum AssertionTypeDescription with a type containing all the assertion descriptions.

I'm not sure how much value the string literals have, but the description has a type now instead of string.

@sindresorhus
Copy link
Owner

What problem does it solve? This would be a breaking change as the existing type is exported.

@bjornstar
Copy link
Contributor Author

The arguments are the same as #113, I believe the only reason the AssertionTypeDescriptions are exported is for the tests. Doing a search for AssertionTypeDescription across all of github does not turn up any usage.

I understand not wanting to make a breaking change for what may seem like cosmetic reasons. Changing a export into a type export reduces the footprint of the interface while improving the developer experience.

I have some spare time at the moment and my intention is to make more substantial improvements to the library, specifically doing named exports and improving the type assertion descriptions for is.any. I feel like it might be a bit much for a single PR and I wanted to get the enums out of the way. Do you have a preference for how I should proceed?

@sindresorhus
Copy link
Owner

Thanks for elaborating. I agree, this is better and it's unlikely anyone is depending on the enum.

@sindresorhus
Copy link
Owner

I have some spare time at the moment and my intention is to make more substantial improvements to the library, specifically doing named exports and improving the type assertion descriptions for is.any. I feel like it might be a bit much for a single PR and I wanted to get the enums out of the way. Do you have a preference for how I should proceed?

Improvements in that area are very welcome. They should separate pull requests. The named export pull request will cause large changes, so I would do that one first, and wait with any other changes until that is merged, to avoid conflicts.

@sindresorhus sindresorhus merged commit bd5dfda into sindresorhus:main Jul 30, 2023
@bjornstar bjornstar deleted the remove-enums branch July 30, 2023 23:42
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.

2 participants