P5-wrapper
Update project structure
Proposed Changes
- Update project folder structure to cleanup
index.tsx
Additional Notes (optional)
This is a proposal for a new structure within the src directory. Happy for feedback or change suggestions as always 😄!
Please ensure exports remain exactly the same in the new index.tsx as those within the current index.tsx.
@yevdyko have you had a chance to look at this one yet? Would be good for the v4 release I think but let me know your feedback so we can iterate if necessary 👍🏻
@jamesrweb Thanks for bringing this up!
I'm not sure this is a good idea for a 120 line library. After such a fragmented structure, it became harder to understand how things work, having to remember what happens in each file instead of evaluating everything in one glance on one screen. I agree that this granularity is good for large projects or with the potential for rapid expansion, which I don't think is the case for this component. So I wouldn't want that change when we have a single line import, each function in separate files, I don't see the benefits of that. I understand that this is a matter of taste, so I don't think my opinion is decisive here. What do you find advantages to the new structure other than defragmenting into separate files?
I think the main benefit of doing it this way is being more explicit about the borders and using explicit import export statements for domain specific areas like the contracts for example. Mixing it all together is nice in one sense but it can also be confusing when you have to scroll up and down all the time. I am easy either way also but I feel the downsides are balanced with the ups and we can always reorganise later if necessary.
Okay, how about putting all the contracts in one file named contracts.ts, for example. And place the main component ReactP5Wrapper in the file index.ts and not in a separate one, since the main logic is there. How about that?
@jamesrweb Thanks for bringing this up! I'm not sure this is a good idea for a 120 line library. After such a fragmented structure, it became harder to understand how things work, having to remember what happens in each file instead of evaluating everything in one glance on one screen. I agree that this granularity is good for large projects or with the potential for rapid expansion, which I don't think is the case for this component. So I wouldn't want that change when we have a single line import, each function in separate files, I don't see the benefits of that. I understand that this is a matter of taste, so I don't think my opinion is decisive here. What do you find advantages to the new structure other than defragmenting into separate files?
I think the main benefit of doing it this way is being more explicit about the borders and using explicit import export statements for domain specific areas like the contracts for example. Mixing it all together is nice in one sense but it can also be confusing when you have to scroll up and down all the time. I am easy either way also but I feel the downsides are balanced with the ups and we can always reorganise later if necessary.
Okay, how about putting all the contracts in one file named
contracts.ts, for example. And place the main componentReactP5Wrapperin the fileindex.tsand not in a separate one, since the main logic is there. How about that?
Contracts by definition should be separated though IMO. I also like the components being seperated from one another. I guess for me it is either do it this way or leave it as things are. I kind of like this way though but happy to not do it if theres a good reason. I don't know, personally I think it's fine how it is done here after re-reviewing it.
Maybe we move the Memo from index.ts into a component though and just export that like the other exports then, what do you think of that as an idea for example?
Lets keep iterating until we get it right 🚀
I don't think these one-line files make sense to me:
It's like using design patterns for the sake of patterns themselves, it seems to me we can spread everything over several files, but such granularity as we have in the current PR has no advantages.
Why not have the main component in the index if the entire library consists of that component?
I don't know, I think that there are no disadvantages either and concerns are now cleanly separated in some form from one another. I also don't think it makes sense to abstract one component but not the other. You know what I mean? Consistency in whatever form is better than partial adherence.
@jamesrweb could the last commit be a separate PR because it's related to the structure topic?
@jamesrweb What about having 3 files: index, 2 component files and all related contracts inside the particular component file? This way we improve the structure, but avoid unnecessary abstractions and one-line files, which don't add value. How's that for a compromise?
We can also extract a huge comment from the index file in the repo issue, since that is the right place for it. WDYT?
@jamesrweb I guess this dependencies update script doesn't seem to work, does it?
@jamesrweb could the last commit be a separate PR because it's related to the structure topic?
The whole PR here is structural, right?
@jamesrweb What about having 3 files: index, 2 component files and all related contracts inside the particular component file? This way we improve the structure, but avoid unnecessary abstractions and one-line files, which don't add value. How's that for a compromise?
We can also extract a huge comment from the
indexfile in the repo issue, since that is the right place for it. WDYT?
The reason it is in the code was actually because I thought that if anyone pulls or does cmd+click to view the source code then it would be clearer as to why we use as to stop anyone wasting time trying to remove it when it's unfixable thanks to TS itself 😅🙈. IDK, we could abstract the comment to an issue and then just link that issue in the code though I suppose, WDYT?
@jamesrweb I guess this dependencies update script doesn't seem to work, does it?
Seems not to even be triggered, I think GH is either having an issue or we didn't configure something 😥🤨... will look into it on a different PR / issue pair.
I saw your message about using renovate, I'll take a look and consider it over the weekend.
@jamesrweb What about having 3 files: index, 2 component files and all related contracts inside the particular component file? This way we improve the structure, but avoid unnecessary abstractions and one-line files, which don't add value. How's that for a compromise?
We can also extract a huge comment from the
indexfile in the repo issue, since that is the right place for it. WDYT?
I would say that we either move in this direction or we keep it as it is.
I'm leaning to make the change but I'm mindful of your opinion and will consider it deeply. Perhaps a good middle ground would be to get another reviewer from the community to mediate, WDYT?
I want to keep everyone happy but eventually a decision has to be made and documented.
I'm not bothered what the decision is even if I have a bias right now, as long as it's reasoned, acceptable to all sides and clearly laid out.
Another thing to consider is of course contributors and end users. I think this new way is a bit clearer and cleaner TBH and will make code splitting far simpler for the v4 implementation but again, that's all subjective, right?
Let me know your thoughts either way 💭!
I would like to merge this in preparation for the v4 release. If there is a technical issue or something really blocking this then I am open to hearing it but if it is just a taste subject, which it seems to be, then I think the community should decide and if they really want it rolled back we can always do a minor release accordingly. WDYT?
@yevdyko can you review this within the coming week please?
Please also note issue #207 which is now linked appropriately in the files ready for version 4 being released once this is merged.