Shopify / shopify-app-template-remix

374 stars 151 forks source link

update webhooks organization #855

Closed TheGreatSimo closed 1 month ago

TheGreatSimo commented 2 months ago

Why are these changes introduced?

Organizing webhook routes for better maintainability

What is this pull request doing?

This PR groups all webhook routes into a dedicated webhooks directory improving organization and clarity

Test this PR

shopify app init --template=https://github.com/TheGreatSimo/shopify-app-template-remix#simo

Checklist

matteodepalo commented 2 months ago

Hi @TheGreatSimo, thank you for opening this PR. Could you explain in the description the improvement that this PR brings? Maybe with some before/after scenarios. Thank you.

TheGreatSimo commented 2 months ago

Hi @matteodepalo thank you for your reply

Before

Webhook routes were mixed with other routes, making it harder to manage image

After

Webhook routes are now in a dedicated webhooks+ directory, improving organization and clarity image

This makes it easier to locate, modify, and maintain webhook routes as the project scales

Arkham commented 1 month ago

Just a quick question, why is the folder named webhooks+? it might be a convention I'm not familiar with :)

TheGreatSimo commented 1 month ago

@Arkham I named the folder webhooks+ to take advantage of Remix flat routing This lets me keep the webhook files short without needing to prefix each one with webhook

I also added the Remix flat route dependency If that goes against the repo philosophy I can change it to a regular directory with files that start with webhook Let me know what you think!

byrichardpowell commented 1 month ago

Hey @TheGreatSimo 👋

Thanks so much for the PR.

I personally really like putting all the webhook routes in a single folder. Unfortunately, I'm a little nervous about adding the Remix flat routing package (Even though I like it). We try to keep the template fairly vanilla Remix, which means avoiding adding packages that aren't what 90% of developers would expect most of the time. It's a difficult balance because adding something might make the template better for some people, but it might be something that other people want to rip out. Adding something can also create a disconnect with the official Remix docs too which describe it one way, but the template does it slightly differently.

Can we do this without the Remix flat routing package?

TheGreatSimo commented 1 month ago

Hey @byrichardpowell thank you for your feedback I’m not sure if there’s another way to organize routes in Remix v2 without it If you have suggestions I’d really appreciate them otherwise feel free to close the PR if it doesn’t fit

byrichardpowell commented 1 month ago

Thanks for understanding @TheGreatSimo :+1:

Unfortunately I'm not aware of another way, so I'll close this PR. Thanks so much for contributing though. I really appreciate you taking the time to submit this.