Skip to content
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: Angular migration path #17767

Closed
wants to merge 41 commits into from
Closed

POC: Angular migration path #17767

wants to merge 41 commits into from

Conversation

diosmosis
Copy link
Member

@diosmosis diosmosis commented Jul 14, 2021

EDIT: broke the formatting by accident

Description:

This is a complete (for some definition of "complete") PoC of using Angular to replace AngularJS directives & components in Matomo, while still running alongside AngularJS. It can be used as a base to compare w/ the other PoCs that are coming and to see a possible Angular migration path.

Current State

There are JS errors when using the selector and I didn't test everything (just the absolute minimum functionality):

(Also a note re my knowledge of Angular: I'm somewhat familiar w/ the concepts and way of working in the framework, but I wouldn't call myself an expert. It's been several years since I worked with Angular last.)
Core Concepts

Components vs MV-whatever

Components in Angular encapsulate UI state and are primarily state driven. They are different from AngularJS directives, which are primarily mechanisms to manipulate the dom based on data. Components are designed (by me at least) by thinking about defining the state that's managed first (the list of sites displayed, the currently selected site, the search term), defining the input used to vary how the component behaves/looks, the output to allow outside elements to know when something in the component changes. Then filling out the implementation details.

Contrast w/ angularjs where it's more imperative. There's the input data, then the logic to render it and do things when data changes. Thinking about the interface and having hard boundaries between directives and the state they manage isn't considered. Nothing is there to prevent the creation of side effects in the input data.

Observables

Observables are a bit like promises, but instead of emitting one value to .then(), they can (potentially) emit any number of values. So they're a stream of data. They're useful in a UI context because they let you define a UI by thinking about how data flows through it.

Some definitions:

  • observable: a stream that can be observed. we can listen to and act on the data that comes through.
    observer: something we can send values to.
    subject: both an observable and an observer. we can emit values to a stream, and act on the values we send. we do both in different places in the code, which is what makes this useful.

    For example:

    • we create a subject for the list of sites to display
    • we define our ui based on the sites subject (transforming the value of the subject, the list of sites, to whatever we want, storing the transformations in other observables)
    • then subscribe to these observables in our template, so the template automatically updates when they change.
    • and when the user types into the input, we query for new sites, then emit the value in the subject. this automatically passes through the transformations and automatically updates the template.

In this workflow, change detection (determining when a property changes, figuring out what needs to be re-rendered and then re-rendering) isn't used. As in, we never need to use $watch, $apply, $timeout or even think about those things. (That's not to say change detection is not used at all, templates are still re-rendered when @input() values for example change, but we don't just $watch an expression, check it over and over in the digest cycle and re-render things when it changes).

There is a learning curve for observables, and you don't have to use them to use Angular. It's definitely possible to just do things imperatively.

And a quick note: we can't get an observable for @input values. This was a requested feature when I last used Angular several years ago, and is still an open issue in Angular. But it's easy to workaround if needed (as done in this PoC).

Modularization

Angular is a modern framework library, so it is expected it will be compiled and built. Getting around this is harder than figuring out a way to work w/ it (in my experience).

This poses a challenge for the way Matomo achieves modularization since we can't just include some JS files that use Angular directly to work like we do with AngularJS. Well, we probably could but it would be a lot slower. We also can't use TypeScript making the code far more verbose.

In the PoC, every plugin can have it's own angular library in the angular subfolder. The library is compiled via a command and we load the .umd.js file. (NOTE: this can be automated so users don't have to use the getJavaScriptFiles event). When a plugin is deactivated, the angular library will no longer be loaded.

There are two commands to facilitate this:

* coreangular:generate-angular-module: this generates a blank library with an example component in a plugin folder library.
* coreangular:ng-build: this compiles a plugin library.

NOTE: it is required to use this commands, using ng (the Angular CLI tool) directly will not work. The library it generates will not go into a plugin directory, it will go elsewhere, and it will be incorrectly generated. There appears to be a bug somewhere since Angular will not recognize some of the generated library files because the entrypoint is in a sibling subfolder and not in the parent folder of the actual library files. This is why the libraries don't have a public_api.ts file which is generated with ng generate library.

And ng build depends on a root angular.json file that holds a manifest of every library in the plugin. But the number of angular projects depends on the plugins that have them. So the file needs to be generated dynamically before ng build is used.

Sidenote: we can also write a watch command to make development a bit easier.

Migration Path

After the bugs in this PR are fixed, Matomo could be converted to Angular one component at a time.

Models and services can be converted when a component uses them. We can delete them only if they are not shared among other angularjs code. If we keep converting they will eventually all be removed since the dependent code will slowly decrease to zero.

Twig templates should also be replaced. Instead, we output pages w/ an Angular component to encapsulate the UI. This'll make it easier to get rid of the AngularJS directives, since we'll be able to use the Angular version in more places.

Maintaining backwards compatibility

Angular does not allow using attribute directives when downgrading components (they expected everyone to switch to AngularJS components I'm guessing). But we can still provide backwards compatibility by creating a thin wrapper around a component (an example in this PR).

This also makes it possible to provide two way binding in AngularJS to avoid breaking things, without using it in Angular (which somewhat defeats the purpose of using Angular). We can listen to events in the adapter and set values we bind to the adapter.

Before releasing Matomo 5, we can get rid of all the adapters, downgrades and get rid of AngularJS completely.

Migration Phases

  • Phase 1 - convert all angular directives and related code to Angular. Each converted directive/component should have an associated adapter that keeps existing code backwards compatible. This should be done in all plugins as well. When a higher level component that uses other components is converted, we'd change the use of the dependent components to Angular, slowly removing angularjs code.

  • Phase 2 - convert all twig templates that use angular into Angular components and just render the new component in twig. Ideally, we'd only use twig to generate the containing HTML.

  • Phase 3- remove shared angularjs files and with the Matomo 5 release, remove all angularjs code + downgraded adapters.

Other implementation details/challenges

* Script loading order: Angular expects its scripts to be loaded after the page is rendered, and we need all our angular modules loaded before we bootstrap the angularjs app. But we have inline scripts everywhere that expect jquery to be available, angular to be available, and the piwik angularjs app to be bootstrapped (since we immediately try to access the angularjs injector in some places). We can't load the Angular files last, because then the code that uses the injector will fail. We can't do it first, because then we bootstrap the app too early.
Worked around quickly in this PoC, by making every script, even inline ones defer and wrapping inline scripts in a DOMContentLoaded event. Seems to work

* Script loading order: Angular expects its scripts to be loaded after the page is rendered, and we need all our angular modules loaded before we bootstrap the angularjs app. But we have inline scripts everywhere that expect jquery to be available, angular to be available, and the piwik angularjs app to be bootstrapped (since we immediately try to access the angularjs injector in some places). We can't load the Angular files last, because then the code that uses the injector will fail. We can't do it first, because then we bootstrap the app too early.
Worked around quickly in this PoC, by making every script, even inline ones defer and wrapping inline scripts in a DOMContentLoaded event. Seems to work.

* Transclusion from angularjs to a downgraded adapter, to an directive is possible via programmatically getting the transclude content and inserting it into an element in the child Angular component via jquery. Two examples exist in this PR. The only possible issue is there is a slight flash before the transclusion takes effect (it's possible this is only seen in a development build, but I didn't check this). The flash would go away when using the Angular components directly, so it would be a temporary change.

Pros & Cons

* Pros
    * TypeScript, with easy to install and use polyfills. Makes development much faster.
    Will attract developers (who would see AngularJS and vanilla JS in 2021 and think "I want to work on THAT project!"?).
    The ng compiler tool seems stable and does not need much configuration. There was one issue described above, but it was fairly easy to workaround. It's definitely easier than using webpack directly.

* Cons

    * JS errors can be hard to debug, especially w/ some of the dynamic things we do. Eg, I'm currently seeing an error when putting a downgraded component in a widget that is something like "expected to be outside of angular but isn't!" which has very little context.

    * the downgrade functionality does not appear to be supported well. I suspect due to the amount of time since Angular was released. For example: there seems to be a bug, or just weird quirk in https://github.com/angular/angular/blob/cd2d82a91a5f547ae4c3d369f7f777245324c9e5/packages/upgrade/src/common/src/downgrade_component.ts#L94-L95. If you just downgrade components, w/o modules, everything will work. If you downgrade multiple modules and add them to an angularjs app, everything will also work. But if you only do it for ONE module, like say a single module in a proof of concept, downgraded components won't work, because the referenced code will not look for the correct Angular module to look for the component in. This is also an example of hard to debug errors. Though this might be a special case.

    * Complexity of concepts involved (if using Observables for example). This could also result in attracting better developers, since good developers tend to find complex ideas worthwhile.

…w defer merged asset (that uses defer in the script element). not yet integrated w/ ExampleAngular library.
@diosmosis diosmosis added the RFC Indicates the issue is a request for comments where the author is looking for feedback. label Jul 14, 2021
@diosmosis
Copy link
Member Author

The PoC now contains more downgraded components, including content-block and enriched-headline, both of which require transcluding from angularjs to angular.

@sgiehl sgiehl added the Do not close PRs with this label won't be marked as stale by the Close Stale Issues action label Aug 6, 2021
@tsteur
Copy link
Member

tsteur commented Aug 25, 2021

fyi closing these for now. We'll still have the code just in case

@tsteur tsteur closed this Aug 25, 2021
@sgiehl sgiehl deleted the angular-migration2 branch January 9, 2024 16:08
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants