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: Typescript fixes #121

Merged
merged 6 commits into from Sep 8, 2021

Conversation

SimpleCreations
Copy link
Contributor

Overview

@mmazzarolo
Copy link
Owner

Thank you @SimpleCreations ! 🧡
I'm taking a look at it, mind fixing the linting error in the meanwhile? (Otherwise, I'll fix it later — not sure why Husky didn't fix it)

Copy link
Owner

@mmazzarolo mmazzarolo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code is looking solid.
Thank you for going above and beyond the TypeScript fixes!

I'll give it a quick run in a couple of hours and then I'll release it.

Wondering if this should be a major bump or not.
At a first glance, I'm not seeing cases where we're making existing types more strict. But normally, when we do these many type changes in a single shot, I prefer doing major releases to ensure we don't break existing build type-checks.

What do you think?

buttonSeparatorStyle,
]}
/>
);
Copy link
Owner

@mmazzarolo mmazzarolo Sep 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, I think I haven't read this flow in ages, it's 4 years old — no idea why the strings were hardcoded in the first place, haha.
Thank you so much for cleaning this up!

@SimpleCreations
Copy link
Contributor Author

Wondering if this should be a major bump or not.
At a first glance, I'm not seeing cases where we're making existing types more strict. But normally, when we do these many type changes in a single shot, I prefer doing major releases to ensure we don't break existing build type-checks.

What do you think?

I don't see a major version bump being necessary because yes, seems like we're only making the types less strict.

However I don't have experience maintaining public npm packages so I'm not sure about the potential pitfalls.

@SimpleCreations SimpleCreations changed the title Typescript fixes fix: Typescript fixes Sep 8, 2021
@mmazzarolo mmazzarolo merged commit 1ba6f16 into mmazzarolo:master Sep 8, 2021
github-actions bot pushed a commit that referenced this pull request Sep 8, 2021
# [9.0.0](v8.2.0...v9.0.0) (2021-09-08)

### Bug Fixes

* Typescript updates ([#121](#121)) ([1ba6f16](1ba6f16))

### BREAKING CHANGES

* Updated TypeScript type definitions. Types _should_ just be more relaxed now — but since we updated the entire codebase we'll release this a major bump to be safe.
@github-actions
Copy link

github-actions bot commented Sep 8, 2021

🎉 This PR is included in version 9.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants