magento / pwa-studio

šŸ› Development tools to build, optimize and deploy Progressive Web Applications for Magento 2.
https://developer.adobe.com/commerce/pwa-studio/
Open Software License 3.0
1.06k stars 682 forks source link

[feature]: Replace react-router-dom instances with @magento/venia-drivers to support "bring your own router" #2479

Closed paales closed 4 years ago

paales commented 4 years ago

Is your feature request related to a problem? Please describe.

So, we're trying to create a NextJS compatible implementation to use the venia-ui components in our application. To be able to do that, we need to have a router that is compatible with react-router-dom.

I believe there is an intention to support this by having @magento/venia-drivers implementable by the user. However, it seems that throughout the codebase there are lots of places where react-router-dom is used directly, which is causing issues.

Describe the solution you'd like To solve this issue I have a few ideas;

In an ideal scenario the exposed API should be reduced / simplified as well: Create a custom API that is implemented with react-router-dom and next/router routing solution.

In my opinion the driver enable users to use peregrine/venia-ui components inside non react-router-dom environments.

Describe alternatives you've considered I've tried to override the @magento/venia-drivers but that doesn't help, because there are direct references to react-router-dom

Additional context

Please let us know what packages this feature is in regards to:

m2-assistant[bot] commented 4 years ago

Hi @paales. Thank you for your report. To help us process this issue please make sure that you provided sufficient information.

Please, add a comment to assign the issue: @magento I am working on this


awilcoxa commented 4 years ago

Created PWA-726 in internal Jira for review

zetlen commented 4 years ago

Hi @paales . Once again, our turnaround is slow--especially mine, because I'm working mostly on extensibility. But this is a very good suggestion and an important thing to talk about. I'm going to start with your problem, because I have an initial question, but then I'll talk about the situation with @magento/venia-drivers.

Your problem

You want to put components that use react-router inside a NextJS app. In this case, you want to use Venia components, but from my research I see that this is difficult to do with any React Router stuff. The NextJS router is not drop-in compatible with React Router and is not meant to be--the main useRouter hook itself is a NextJS idea--and Google shows me several experiments to try and integrate the two, with limited success.

Have you had success doing this? Outside of a PWA Studio project, have you been able to integrate components which expect a React Router into a NextJS app?


That's my question for you. Notwithstanding that, here's the rest of the brain dump on the drivers concept.

We added drivers to make it easier for non-PWA-Studio projects to use Venia components. It's still a work-in-progress. Venia UI is tightly bound to the Peregrine architecture and to the application contexts, so it's not clear at this time how independent of PWA Studio we can expect Venia UI to become.

However The React developer community doesn't generally expect that a third-party component will be opinionated about router technology. Some of the most popular component libraries have dedicated APIs for supporting third party routers:

These UI libraries don't have built-in ecommerce logic like Venia UI does. Maybe this exposes a problem with the definition of Venia UI as a module: it's hard to have a) ready-made workflows AND b) totally reusable components in the same library.

For now, the Venia UI roadmap has been much more focused on a) than b), which is why some components have started bypassing the drivers. We can open a PR to fix the direct references in the codebase today, but I think it's worth talking about the future of Venia UI portability--maybe the reusable components are their own library?

For now @paales I recommend aliasing react-router-dom directly in NextJS build config. If Venia's usage of react-router-dom is not drop-in compatible with the next/router then you could write an adapter. If you like, you can open the PR to replace the direct references, but because the future of drivers is unclear, we can't make it a priority.

Quick brainstorm of other things that might be worth exploring:

I'm closing this because I recommend that you alias your router directly in your project today. But if you want to submit the PR to change the references, please feel free to reopen it.