New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PoC: React migration path #17802
Closed
PoC: React migration path #17802
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
diosmosis
added
the
RFC
Indicates the issue is a request for comments where the author is looking for feedback.
label
Jul 22, 2021
If you don't want this PR to be closed automatically in 28 days then you need to assign the label 'Do not close'. |
github-actions
bot
added
the
Stale
The label used by the Close Stale Issues action
label
Aug 6, 2021
diosmosis
added
Do not close
PRs with this label won't be marked as stale by the Close Stale Issues action
and removed
Stale
The label used by the Close Stale Issues action
labels
Aug 6, 2021
fyi closing these for now. We'll still have the code just in case |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Do not close
PRs with this label won't be marked as stale by the Close Stale Issues action
RFC
Indicates the issue is a request for comments where the author is looking for feedback.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
This is a complete (for some definition of "complete") PoC of using React to replace AngularJS directives & components in Matomo, while still running alongside AngularJS.
Like the Angular PoC it migrates the site selector, enriched headline and content block directives, + any related directives/components.
Current State
This one is more functional than the Angular proof of concept. The site selector works without errors or (obvious) issues, as do the other components. It took much less time to code. The only part that is unfinished is the build system, which was not already prepackaged as with Angular. It works, but is not too flexible and will need to be modified (which is doable and shouldn't be too time consuming, but it is more work than w/ Angular).
Core Concepts
Same as with Angular, components are used here to encapsulate parts of the UI. The only difference is where Angular allows many different options, components, directives, pipes, in React there are only components. Everything you can do in Angular you can do in React, it just takes less thinking. For example, the 'focus-anywhere-but-here' directive in the Angular poc is just a component here. The translate pipe in the Angular poc doesn't even exist, we just use
_pk_translate()
directly (since it's all javascript this is fine).The small footprint and set of responsibilities React covers means we can implement other things how we want or need, and do not need to be afraid of it breaking in a future release. Routing, for example, is completely out of scope of React. There are libraries to deal with it, but we don't have to use them. And if we keep our existing routing, and update React, nothing will break.
This also goes for having a solution to automatically initialize react components in twig templates. If we choose to keep that part of matomo, and create such a mechanism, updating react is very unlikely to break it. On the other hand, Angular has already deprecated entryComponents, so we have reason to fear future Angular releases.
Another thing to note is that services aren't a special thing here. It's all just javascript, so we create a class for the service and create a new instance to consume it or consume a singleton, as we would in php. Nothing special needed to mark something as a "service".
Finally, this isn't really a React thing, but with companion libraries like redux can allow us to separate component state (what is owned by the component) from application state (what must be kept in sync across components). None of this is truly needed though, and matomo probably doesn't need it.
Modularization
Modularization is achieved in a way similar to the Angular PoC. Each plugin can have a react subfolder that compiles to a UMD. They are loaded if the plugin is activated.
Unlike the ANgular PoC, loading order does not matter, and we do not need to bootstrap the application with any extra modules.
Again two commands are supplied:
It's not needed to use these commands as it is with Angular, but it's far more convenient. Any library that compiles with our webpack config can be used. And you can run the build script directly in any react folder if you want.
The Build System
The build system here uses webpack. It uses an ejected configuration from a create-react-app app modified for this specific use case. Unfortunately, our use case is far away from most react apps, so this may not have been the best approach.
Currently it works, but there's a lot there that we don't need, and a lot we'd need to modify to make sure we have optimal builds (for example, right now parts of core js appear in every plugin build, even though we include it in CoreReact.php).
Migration Path
The migration path is the same as in the Angular PoC. The process of rewriting is the same and we maintain backwards compatibility in the same way. We just don't need to do any sort of downgrading.
Other implementation details/challenges
Build system: Again, the build system is the primary challenge. We'd need to spend some time making sure it works how we want. This can be done in parallel to conversion work.
Transclusion: Adding angular elements to React is not straightforward. React elements are wrapped HTMLElements, created via the
React.createElement
function. The elements angular creates when transcluding and rendering are not wrapped this way and I can't see a way to wrap them after they are created. So we can't just add them via the normal React mechanism (this.props.children
). Instead there is a TranscludeTarget component that manually appends them to a node. This approach appears to work, but I'm unsure whether it will work w/ more complicated transcludes.Keeping twig templates: We can sort of do this, but not to the extent we currently do. Right now we could, for example, add a
then have React initialize it. Any HTML we put in side would be lost, unless we keep the same transclusion hack that's there now.
Pros & Cons
Pros
Cons