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

fix: 🧪 Replace any with more refined types #243

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

Beraliv
Copy link

@Beraliv Beraliv commented May 12, 2024

Goal

Hey @lukeed! It took me some time to create a draft.

Please let me know if you'd like to see dependencies like typescript and ts-essentials (I don't insist but some utilities may be handy but I will leave it up to you)

Related to #169

Summary

  • Added typescript@^4.5 as devDependency to be able to run type tests
  • Added ts-essentials as devDependency to use DeepPartial e.g. for equal (TBC)
  • TBC (it's still in progress)

Apologies for

Planned

  • Replace types for ok, equal, type, instance, not
  • Added type tests for all changed declarations

@lukeed
Copy link
Owner

lukeed commented May 12, 2024

Hi there, there is zero reason to add typescript (or essentials) as a depending here. At best, they’d be devDependencies, but still I don’t even see need for them there either. A “dependency” means everyone will forcibly download it at installation because X is needed at runtime.

@lukeed
Copy link
Owner

lukeed commented May 12, 2024

The ArrayOrSingle type is exactly what I have already. Please remove, pointless change

@Beraliv
Copy link
Author

Beraliv commented May 12, 2024

@lukeed hey! Thanks for a quick response!

there is zero reason to add typescript (or essentials) as a depending here. At best, they’d be devDependencies, but still I don’t even see need for them there either

I agree with you. I'm planning to add type tests to make sure there are no regressions, therefore you would need typescript to be listed as devDependency.

The ArrayOrSingle type is exactly what I have already. Please remove, pointless change

No problem, addressed it in abeebd1

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