anza-xyz / solana-pay

A new standard for decentralized payments.
https://solanapay.com
Apache License 2.0
1.29k stars 450 forks source link

Refactor context #138

Closed tranminhquanq closed 2 years ago

tranminhquanq commented 2 years ago

Right now I'm seeing React Context code written in hook files, so I made this pull request to include them in Context related files for clarity

vercel[bot] commented 2 years ago

Someone is attempting to deploy a commit to the Solana Team on Vercel.

A member of the Team first needs to authorize it.

vercel[bot] commented 2 years ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
solana-pay-docs ✅ Ready (Inspect) Visit Preview Jul 9, 2022 at 5:09PM (UTC)
jordaaash commented 2 years ago

This is intentional. The context state interface is needed in the hooks without a reference to the implementation (the context provider). We have components (TSX files) consuming hooks (TS files), not the other way around.

Some other ways to refactor this are to

Thanks for thinking about these things! I'm going to close this because it's a bit hard to review and doesn't clearly improve on the maintainability.

tranminhquanq commented 2 years ago

hmmm, I still think we should separate them 🥲 cc @jordansexton

jordaaash commented 2 years ago

What is gained from separating them? And which of the approaches do you think is best?

Important to remember that this point of sale app is just a proof of concept and that it's already largely served its purpose. It was never intended for production use or long term development.