RFC: Make React the Primary Framework #32
Conversation
jonathandick
requested review from
denniskigen,
donaldkibet,
ibacher and
manuelroemer
There was a problem hiding this comment.
Thanks for this! So, we've obviously ended up in a place where React is our primary framework, at least de facto if not de jure and embracing that seems like a good idea.
I think one concern I have here is around vendor lock-in, which is, of course, always a risk. My concern is if, say, React were to stop being actively developed in the next, say, 5 years, we might be stuck with an app that now needs a full rewrite. At least with Single SPA, as you've shown, the shims that we'd need to replace it are fairly minor and we could start swapping out individual components with whatever the new framework is. By comparison, the migration path from React seems like it will be more complicated: essentially re-writing parts and then re-exposing them as thing React components.
This isn't so much an objection to going forward with this, just something that it feels like needs to be at least thought-through in part.
|
|
||
| Since most of our components (>= 95%) are React components and since in React its very easy to have a single rendering tree dispatching to multiple DOM nodes (using the `createPortal` API of React) we should consider going to a single rendering tree of React. The other advantage is that the framework agnostic approach has problems with otherwise simple things such as prop updates. While this is fully integrated and highly optimized in React, the framework agnostic approach right now makes the update inefficient. | ||
|
|
||
| Besides the performance part we would also see significant reduction in complexity. Right now developers need to think in multiple dimensions. For instance, the routing is happening on an *outside* level (using `single-spa`s router) and potentially even on an *inside* level (using, e.g., the `react-router`). While most of the time this works as expected, sometimes weird behavior is observed. |
There was a problem hiding this comment.
sometimes weird behavior is observed
Do you have an example of this? For historical purposes, it's useful to have concrete examples of why we're rejecting an approach that we've pursued.
There was a problem hiding this comment.
It seems like this is the main argument in favor of this change. As such I think it should be very clearly and concretely explained.
|
|
||
| There are some ways we can go alternatively, including: | ||
|
|
||
| 1. As-is, but with enhancements to the `single-spa-react` connector (would have several downsides and would not provide all the optimizations we look for) |
There was a problem hiding this comment.
For instance still having a router of router, which degrades developer and user experience.
There was a problem hiding this comment.
| 1. As-is, but with enhancements to the `single-spa-react` connector (would have several downsides and would not provide all the optimizations we look for) | |
| 1. Make enhancements to the `single-spa-react` connector. This would eliminate the unnecessary React render trees and allow React to handle props directly. Routing, however, would remain as it is currently. |
| 1. As-is, but with enhancements to the `single-spa-react` connector (would have several downsides and would not provide all the optimizations we look for) | ||
| 2. Restricting frontend modules to React exclusively, so any module with components in another language needs either to convert or custom mount their framework / application | ||
|
|
||
| The first way would be half-baked again, and comes with some drawbacks (frontend modules would still require recompilation here). The second way would be too much effort on the development side of frontend modules. Ideally, the current development model should not be interrupted in any way / significantly by this proposal. |
There was a problem hiding this comment.
I would migrate these explanations into the text of the alternatives above. I'd also drop the "half-baked" language. Just explain the drawbacks.
|
|
||
| Such a generic higher-order component can be used to create converters, e.g., to bring Angular components to that world (where React is the primary framework). | ||
|
|
||
| Another thing to consider is the use of `single-spa` converters such as `single-spa-react` or `single-spa-angular`. While the former is actually already hidden by default the latter is used, for instance, in the forms frontend module. |
There was a problem hiding this comment.
Ideally, an RFC should be explaining the decision and the way forward. Which of these various mechanism of integrating components from other frameworks do you recommend?
There was a problem hiding this comment.
The implementation guide was added by explicit demand.
|
|
||
| The `createLifecycle` could be very similar to the existing implementation. Alternatively, the original `single-spa-angular` is kept in place, and instead the `makeConverter` is called implicitly in these scenarios. | ||
|
|
||
| Finally, the use of some APIs such as `navigate` needs to be adjusted. |
There was a problem hiding this comment.
Because they work against the router from single-spa.
There was a problem hiding this comment.
That makes sense, @FlorianRappl . A related question: how will non-React apps trigger navigation? e.g. if I have an Angular microfrontend that links to the home page, how does it work?
There was a problem hiding this comment.
history is framework agnostic.
|
Thanks @FlorianRappl , this is thoughtfully done. And thanks @ibacher for asking excellent questions. I think those are key to being able to really evaluate this. I would submit an additional alternative to the ones posed here. Make pages and extensions aware of each other's frameworks, so that at extension mount time, if they are in the same framework, the framework-native mounting can be used. This would not require implementing or changing any shims, and would eliminate the superfluous React trees within React trees (and, optionally, all the OpenMRS Root Decorator layers at the extension boundary—contexts, suspense boundary, etc). Routing would remain as it is currently. |
Co-authored-by: Ian <52504170+ibacher@users.noreply.github.com>
Co-authored-by: Ian <52504170+ibacher@users.noreply.github.com>
Not sure what you mean. All the MFs would need to be rewritten then anyway and as written, "primary framework does not mean exclusive framework". You can always exchange the transport mechanism. It does not matter if the primary framework is React or Angular. That could be swapped out.
I think this also missed the point. We will always have a convenience mechanism which is capable of transforming an arbitrary lifecycle to the lifecycle of the primary framework.
No that is not what this proposal is about. It's about making React the primary framework, not rewriting components written in other frameworks in React (e.g., AMPATH Forms will remain in Angular). |
| Right now every frontend module registers a couple of components (mostly "extensions" and "pages"). Each component will be rendered using a completely new rendering tree. This comes with some significant overhead as compared to reusing a single rendering tree. | ||
|
|
||
| Since almost all of our components are React components and since in React its very easy to have a single rendering tree dispatching to multiple DOM nodes (using the `createPortal` API of React) we should move to using only a single React rendering tree. The other advantage is that the framework agnostic approach has problems with otherwise simple things such as prop updates. While this is fully integrated and highly optimized in React, the framework agnostic approach right now makes the update inefficient. |
There was a problem hiding this comment.
It would be good to have an empirically grounded estimate of the performance impact of this change—with respect to each of these two inefficiencies.
There was a problem hiding this comment.
the framework agnostic approach has problems with otherwise simple things such as prop updates
Are there examples other than prop updates, or can we simplify this?
There was a problem hiding this comment.
My concern is if, say, React were to stop being actively developed in the next, say, 5 years, we might be stuck with an app that now needs a full rewrite.
@ibacher "true", but the much bigger problem in such a scenario is not so much to change the pieces needed to make another framework primary, rather it will be to rewrite everything else (every O3 MF on Earth) in that other framework.
|
@mksd Yeah, I think that was a very poorly phrased concern. What I was trying to get at is something along these lines: does adopting React as the primary framework have trade-offs vs the reasons we moved to single-spa in the first place, and more importantly, just being clear about what those trade-offs are. I think Florian's comment addressed that concern well enough. |
There was a problem hiding this comment.
I think it's worth considering this. For me, the performance considerations are not as important as the developer experience and organizational considerations, since in my experience a dozen separate React roots on a page is completely fine and hasn't impacted performance in a meaningful way within the applications I've worked on. If performance is the main motivation for this proposal, performance benchmarks with and without single-spa for the openmrs app would be helpful so that the discussion is more objective.
Regarding converters to support multiple frameworks, I agree that single-spa is not the only way, but would recommend against OpenMRS creating its own framework adapter layers, instead preferring either single-spa parcels (framework agnostic components, but without single-spa's router), web components, or the various other open source solutions for doing so (in the old days we used ng-react to be able to embed react into angular 1 components, and now there are a bunch of similar projects for angular 13, vue 2/3, svelte, etc.
Coming back to developer experience and organizational concerns, I agree that retaining framework agnosticism introduces some complexity that can make things harder for developers to get started, diagnose errors, and understand what's going on conceptually. Generally speaking, a good architecture is one where developers are focused on building the product and its features more so than on the technical infrastructure. If that is not the case for developers working on OpenMRS with single-spa then it speaks to either the tooling/documentation not being sufficient or to the complexity being too great. I generally lean towards creating the tooling to make the DX better, and have seen the DX for companies using single-spa to actually be easier to understand and use than for companies with monolithic applications. So I don't think that single-spa is inherently a worse DX.
I think the original decision to use single-spa was based in that Ampath was using Angular 6, Bahmni was using Angular 1, and most other things were using the JSP frontend. The hope was that by creating a framework agnostic architecture that it would allow for as many groups as possible to incrementally migrate to a similar destination architecture, without requiring full rewrites of all of those systems. I don't know if that has been successful, but that was an original intent.
The other intent of framework agnostic architecture / microfrontends was to allow for an easy way for someone to create a frontend application and then later integrate it in with other pieces of OpenMRS, rather than working within a monolithic codebase similar to openmrs-core where it's difficult to make quick progress on new features. The idea was that since there are so many groups who use OpenMRS, some may end up preferring Vue or Angular and so supporting those as first class citizens (rather than second-class under the control of React Router) and so having an architecture that allowed that would be nice. The system for plugging in whatever code you want into OpenMRS was one of the main hopes for the architecture. Doing so is possible without single-spa or with single-spa minus its router, but obviously the original choice was to use single-spa with its router.
So to me this decision boils down to "how important is framework agnosticism - should other frameworks be first class citizens or second class citizens (can't do their own routing)" and also "how problematic is the current developer experience? Are people getting confused by the architecture of it all? If so, should we improve the tooling and documentation or should we switch to a different architecture that does not have single-spa"?
RFC for O3-911.