Remove propTypes definitions
#42364
Comments
nielslange
added
type: enhancement
|
Hi @nielslange I'd like to work on this... it'll be my first issue for woocommerce-blocks.. I'm setting it up on my local
That's all right? |
|
Hello @hritikchaudhary 👋 Thanks for showing interest in working on this issue! Any help is much appreciated. Given that it touches numerous files, I think it might be better to split this issue into individual issues. That way, it's easier to implement the individual issues. Let me quickly convert this issue into an epic and create the necessary individual issues.
If a *.js file is using a
The To remove the interface ProductSearchBlockProps {
attributes: {
label: string;
placeholder: string;
formId: string;
className: string;
hasLabel: boolean;
align: string;
};
}and then change const ProductSearchBlock = ( {
attributes: { label, placeholder, formId, className, hasLabel, align },
} ) => {to const ProductSearchBlock = ( {
attributes: { label, placeholder, formId, className, hasLabel, align },
}: ProductSearchBlockProps ) => {Kindly note that I created the TS interface for demo purposes without looking too much into this case. I noticed that the label |
nielslange
added
type: epic
|
@nielslange sure not a problem, will work on it. |
@hritikchaudhary I just converted this issue into an epic and created all individual issues. You are welcome to pick one of the issues. I suggest starting with the smallest file. Some files are quite big. Converting them from *.js to *.tsx will, most likely, introduce many TypeScript errors. The smaller the file, in terms of lines of code, the less TypeScript errors will be introduced. Thanks again for showing interest in working in this area, and please don't hesitate to ask if you have any questions. 🙌 |
|
Thanks @nielslange |
|
Hi @nielslange, I have completed the setup of the development environment, had to install [email protected] from shivammathur/php. Do you know how to resolve this? or where can I look to solve this? Cause i converted a block file and unit tests and linting is successful but is there any way from UI I can confirm the functionality of that file is intact.. Environment info: |
|
@hritikchaudhary I suggest following the steps as described on Getting Started. As for the steps you described, I'm not sure what exactly you're trying to do. Here's how my local setup looks like:
cd ~/Sites/store/wp-content/plugins
ln -s ~/Plugins/woocommerce-blocksApart from symlinking the WooCommerce Blocks plugin into your development site, you could also directly fork the plugin into the plugins folder of the site. That said, I suggest heading over to Getting Started, which explains how to start contributing to WooCommerce Blocks. |
|
Hi @nielslange |
Keeping the fork of the WooCommerce Blocks repo in
On your screenshot, I see that the WooCommerce Blocks plugin is active, but the WooCommerce plugin is missing. WooCommerce Blocks can be seen as an add-on for WooCommerce. In WooCommerce Blocks, we migrate shortcodes to blocks. Every few weeks, we then merge WooCommerce Blocks into WooCommerce. However, without the WooCommerce plugin, you will not see the functionality of the WooCommerce Blocks plugin.
Sure, you can proceed with the changes. To do that, please select one of the tasks, as listed in the description of this epic. |
|
ohh! got it. |
|
Raised 1st PR woocommerce/woocommerce-blocks#9565 to fix woocommerce/woocommerce-blocks#9514 and woocommerce/woocommerce-blocks#9523 |
Hi @hritikchaudhary, Thanks a lot for your effort on these issues, especially on woocommerce/woocommerce-blocks#9613. I've had a quick look at it and see that you've worked on four different sections:
Your hard work is truly appreciated! You've applied changes to multiple sections, and that shows your dedication. However, just to share some thoughts for future work - it can be a bit easier to manage if we do smaller updates. For example, instead of one big update that includes changes to many sections, we can do smaller updates for each section. Why is this a good idea? Let's imagine this scenario: we put all the changes together, but after a few weeks we discover a problem in one section. If we need to go back to the previous version, we'd have to undo all the changes, not just the ones that caused the problem. On the other hand, if we did separate updates for each section, we could just go back to the previous version of the section with the problem. The other sections wouldn't be affected. I understand that this may seem like more work, and I'm sorry if it seems like I'm asking a lot. But it will make it easier to find and fix any issues in the future. Would it be possible to split woocommerce/woocommerce-blocks#9613 into separate updates for each section? If it's too much work, don't worry, we can keep it as is for now. And for next time, if you could focus your updates on one specific section, that would be amazing! Hope this makes sense and thanks again for all your hard work. You're doing a fantastic job! |
|
Yes no worries I can split woocommerce/woocommerce-blocks#9613 into separate updates one for each issue.. it is not much work. |
|
Hi @nielslange, I was going through reviews module assets/js/blocks/reviews we have 10 files in it but as only 3 files had proptypes on 3 issues are there, I think we should convert all of them to ts/tsx. If you agree can I fix these in one of the three issues we have or should i create new issues for each and add them to this epic?
|
Thanks for double-checking. I'd say, converting all files to .ts/.tsx makes sense. Please go ahead and do that. The next days, I'll be travelling, so I might review your contributions a bit slower. Feel free to slow down a bit. 😀 |
haha sure np.. |
nielslange
added
the
priority: normal
While the majority of files have been converted from JS to TS, we still have a few JS files that use
propTypesdefinitions. In addition, we also have a few TSX files that usepropTypesdefinitions.Steps to reproduce
propTypesin your favourite IDE.Running these steps, I can find 211 results in 36 files.
Expected outcome
I suggest we eliminate all
propTypesdefinitions and utilize TypeScript (TS/TSX) for type checking.To do
editor-container-block.jstoeditor-container-block.tsxand replacepropTypeswith TypeScript definitions woocommerce-blocks#9526with-reviews.jstowith-reviews.tsxand replacepropTypeswith TypeScript definitions #42360editor-block.jstoeditor-block.tsxand replace propTypes with TypeScript definitions #42294Done
block.jstoblock.tsxand replacepropTypeswith TypeScript definitions woocommerce-blocks#9514block.jstoblock.tsxand replacepropTypeswith TypeScript definitions woocommerce-blocks#9515block.jstoblock.tsxand replacepropTypeswith TypeScript definitions woocommerce-blocks#9516block.jstoblock.tsxand replacepropTypeswith TypeScript definitions woocommerce-blocks#9517block.jstoblock.tsxand replacepropTypeswith TypeScript definitions woocommerce-blocks#9518block.jstoblock.tsxand replacepropTypeswith TypeScript definitions woocommerce-blocks#9519edit.jstoedit.tsxand replacepropTypeswith TypeScript definitions woocommerce-blocks#9520edit.jstoedit.tsxand replacepropTypeswith TypeScript definitions woocommerce-blocks#9521edit.jstoedit.tsxand replacepropTypeswith TypeScript definitions woocommerce-blocks#9522edit.jstoedit.tsxand replacepropTypeswith TypeScript definitions woocommerce-blocks#9523frontend-block.jstofrontend-block.tsxand replacepropTypeswith TypeScript definitions woocommerce-blocks#9525frontend-container-block.jstofrontend-container-block.tsxand replacepropTypeswith TypeScript definitions woocommerce-blocks#9527index.jstoindex.tsxand replacepropTypeswith TypeScript definitions woocommerce-blocks#9528index.jstoindex.tsxand replacepropTypeswith TypeScript definitions woocommerce-blocks#9529index.jstoindex.tsxand replacepropTypeswith TypeScript definitions woocommerce-blocks#9530index.jstoindex.tsxand replacepropTypeswith TypeScript definitions woocommerce-blocks#9531index.jstoindex.tsxand replacepropTypeswith TypeScript definitions woocommerce-blocks#9532index.jstoindex.tsxand replacepropTypeswith TypeScript definitions woocommerce-blocks#9533index.jstoindex.tsxand replacepropTypeswith TypeScript definitions woocommerce-blocks#9534index.jstoindex.tsxand replacepropTypeswith TypeScript definitions woocommerce-blocks#9535index.jstoindex.tsxand replacepropTypeswith TypeScript definitions woocommerce-blocks#9536index.jstoindex.tsxand replacepropTypeswith TypeScript definitions woocommerce-blocks#9537inner-block-layout-context.jstoinner-block-layout-context.tsxand replacepropTypeswith TypeScript definitions woocommerce-blocks#9538payment-method-card.jstopayment-method-card.tsxand replacepropTypeswith TypeScript definitions woocommerce-blocks#9540payment-method-error-boundary.jstopayment-method-error-boundary.tsxand replacepropTypeswith TypeScript definitions woocommerce-blocks#9541with-transform-single-select-to-multiple-select.jstowith-transform-single-select-to-multiple-select.tsxand replacepropTypeswith TypeScript definitions woocommerce-blocks#9544propTypeswith TypeScript definitions woocommerce-blocks#9545propTypeswith TypeScript definitions woocommerce-blocks#9546propTypeswith TypeScript definitions woocommerce-blocks#9547propTypeswith TypeScript definitions woocommerce-blocks#9548The text was updated successfully, but these errors were encountered: