Replace propTypes with TypeScript definitions
#9673
Conversation
Sadaf-A
changed the title
nielslange
changed the title
propTypes with TypeScript definitions
nielslange
added
type: refactor
There was a problem hiding this comment.
Thank you so much for your contribution, @Sadaf-A. We really appreciate you working on this. 🙌
I briefly reviewed the PR and noticed that your PR changed the package-lock.json file. This PR should not affect this file. Would you mind double-checking this respectively excluding that file? Usually, that file changes when dependencies are updated, which is not the case in this PR.
Apart from that, I also add a few suggestions below. If you require any further assistance, please don't hesitate to reach out. 😀
| @@ -413,15 +412,18 @@ const ActiveFiltersBlock = ( { | |||
| ); | |||
| }; | |||
|
|
|||
| ActiveFiltersBlock.propTypes = { | |||
| interface ActiveFiltersBlock { | |||
There was a problem hiding this comment.
I'm afraid, this will not work. The component itself is already called ActiveFiltersBlock. Having an interface with the same name, leads to a naming conflict. In other PRs, we usually use the name of the component and add Props to the end. In this case, given that the component is called ActiveFiltersBlock, the interface with the TS definitions would be named ActiveFiltersBlockProps. You can find examples of that on https://github.com/hritikchaudhary/woocommerce-blocks/blob/923f89ef0a2050438d84fd2b4097f8c84d1e0ad5/assets/js/blocks/product-new/block.tsx#L22-L26
| attributes: { | ||
| // Define the properties of the attributes object. | ||
| // Adjust the types accordingly. | ||
| }; |
There was a problem hiding this comment.
I noticed that on https://github.com/woocommerce/woocommerce-blocks/blob/trunk/assets/js/blocks/active-filters/types.ts, we already have type definitions. We could reuse them in this PR. On https://github.com/hritikchaudhary/woocommerce-blocks/blob/923f89ef0a2050438d84fd2b4097f8c84d1e0ad5/assets/js/blocks/product-category/block.tsx#L13 and https://github.com/hritikchaudhary/woocommerce-blocks/blob/923f89ef0a2050438d84fd2b4097f8c84d1e0ad5/assets/js/blocks/product-category/block.tsx#L28, you can find an example of how to accomplish this.
| /** | ||
| * Whether it's in the editor or frontend display. | ||
| */ | ||
| isEditor: PropTypes.bool, | ||
| }; | ||
| isEditor?: boolean; |
There was a problem hiding this comment.
Similar to my previous suggestion, I think it's better if we use the type definitions from https://github.com/woocommerce/woocommerce-blocks/blob/trunk/assets/js/blocks/active-filters/types.ts. As these type definitions don't have the definition isEditor?: boolean; yet, should should add it there.
Fixes #9546
This PR replaces propTypes with TypeScript definitions in block.tsx
Testing
Automated Tests
WooCommerce Visibility
Changelog
#9546 Replace propTypes with TypeScript definitions