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

Disabled: Improve TypeScript code after recent refactor to inert #45252

Closed
wants to merge 2 commits into from

Conversation

ciampo
Copy link
Contributor

@ciampo ciampo commented Oct 24, 2022

What?

Cleanup code after recent changes in #44865

Why?

We should try to avoid using @ts-ignore comments when possible.

In the case of the unit tests, those comments where not necessary thanks to this refactor by @tyxla #44930

How?

Testing Instructions

Screenshots or screencast

@ciampo ciampo added the [Type] Code Quality Issues or PRs that relate to code quality label Oct 24, 2022
@ciampo ciampo requested review from mirka, tyxla and chad1008 October 24, 2022 18:26
@ciampo ciampo self-assigned this Oct 24, 2022
@ciampo ciampo requested a review from ajitbohra as a code owner October 24, 2022 18:26
* @param {Object} config Configuration object.
* @param {boolean=} config.isDisabled Whether the element should be disabled.
* @return {import('react').RefCallback<HTMLElement>} Element Ref.
* @param config Configuration object.
Copy link
Member

Choose a reason for hiding this comment

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

We still need the types here, don't we? The docs build script was failing because of missing param types.

I just pushed a commit to restore them, but let me know if this had some special purpose.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch! We usually remove JSDoc types when we add the TypeScript ones, but in this case it looks like they have to be kept around for the docgen — TIL!

Copy link
Member

@tyxla tyxla left a comment

Choose a reason for hiding this comment

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

Nice 🚀

@ciampo
Copy link
Contributor Author

ciampo commented Oct 25, 2022

Closing as duplicate of #45269 (which also adds a CHANGELOG entry) — Thank you @tyxla anyway for you review!

@ciampo ciampo closed this Oct 25, 2022
@tyxla tyxla deleted the polish/disabled-inert branch November 3, 2022 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants