guacamole-client
guacamole-client copied to clipboard
GUACAMOLE-1085: Migrate frontend to angular
This PR includes the current progress for the migration of the AngularJS frontend to Angular. There is still a lot of work left to do, but I think now would be a good time to get some feedback on the code.
Overview
The new frontend consists of one Angular app and two Angular libraries:
-
guacamole-frontend
- contains the main application
-
guacamole-frontend-lib
- contains components that could be useful when building a custom Guacamole-Angular application
-
guacamole-frontend-ext-lib
- contains services and classes for extensions that provide a clear interface for registering additional routes and field types
The application and both libraries are included in the maven build.
TODOs:
- [ ] any changes and new features after revision 466476412a8a93440bea1121f374e17021380f25 (2023-06-02)
- [x] migrate import module
- [x] migrate
index/services/iconService.js
- [x] migrate
manage/directives/connectionPermissionEditor.js
- [x] migrate
manage/controllers/manageSharingProfileController.js
- [ ] caching of API responses
- [ ] refactor the managed client so that reusable code can be moved to the guacamole-frontend-lib
- [ ] some documentation in the code is missing
- [ ] TODOs in the code
- [x] code style might not be correct all the time
- [x] some styles are not applied properly since Angular's components introduce additional DOM elements
- [ ] the functionality to serve complex extensions from the Java code
- [ ] guacamole-common-js should be copied to the frontend in the build process. At the moment I just included a copy at projects/guacamole-frontend-lib/src/assets/. Are there any plans to release an official NPM package for guacamole-common-js?
- [x] type declarations for guacamole-common-js (
projects/guacamole-frontend-lib/src/lib/types/Guacamole.ts
). They will be replaced by the package @types/guacamole-common-js once it is updated. - [ ] automatic generation of the LICENSE and NOTICE files in the maven build.
The angular build creates a
3rdpartylicenses.txt
file, but I don't really know what to do with it. Any comments on this will be appreciated. - [ ] Do not assume that the application is always deployed at
/guacamole/
Extensions
The biggest challenge in the migration was to build an extension system in Angular that was as flexible as the one in AngularJS.
- Adding and modifying translation strings was mostly handled by the REST API and should work like before.
- Inject additional HTML and CSS should work just like in AngularJS.
- See
apply-patches.service.ts
andstyle-loader.service.ts
inprojects/guacamole-frontend/src/app/index/services/
- See
- More complex extensions can now be implemented as a separate Angular application
- Extensions are loaded as a remote module via webpack module federation and the module federation plugin for Angular. Routes and form field can be added by implementing a bootstrap function. Adding custom Angular components to arbitrary locations will also be possible.
- See
doc/guacamole-frontend-extension-example
for an example. - Note: the module federation configuration file (at the moment hard coded as
projects/guacamole-frontend/src/moduleFederation/mf.manifest.json
) should be generated by the Java server in the future
Implementation notes
- AngularJS services that mostly serve as data containers are replaced by simple classes with instance or static methods. Some of the more complex services are replaced by Angular services.
- Named functions are mostly replaced by arrow functions.
- Functions that return a promise with data from REST API are mostly replaced by functions that return an RxJS observable.
- AngularJS's directives are replaced by Angular...
- components if they are configured with
restrict: 'E'
, - directives if they are configured with
restrict: 'A'
.
- components if they are configured with
- All styles are defined globally so that they can be overritten by extension styles
- The authentication service method to perform HTTP request with the current token (
AuthenticationService.request()
) is replaced by a HTTP interceptor (auth/interceptor/authentication.interceptor.ts
). - The error handling of the HTTP requests (
rest/services/requestService.js
) is replaced by a HTTP interceptor (rest/interceptor/error-handling.interceptor.ts
). - The configuration of the $http service (
httpDefaults.js
) is replaced by an HTTP interceptor (index/config/default-headers.interceptor.ts
). - To disable certain interceptors for specific requests, the HttpContextTokens in the
InterceptorService
can be used. - angular-translate is replaced by transloco (https://ngneat.github.io/transloco/).
- A noticeable difference is that transloco uses double curly braces by default to link to other translations keys
instead of the
@:
prefix. However, I change it to@:...:@
to prevent transcloco from removing parts of ICU messages (https://github.com/ngneat/transloco/issues/621).
- A noticeable difference is that transloco uses double curly braces by default to link to other translations keys
instead of the
- I refactored the GuacFileBrowser to simply use an *ngFor loop. In my tests I didn't notice any performance issues.
- I added Cypress for E2E Tests in the
cypress
folder. The tests are not integrated in the build process, and I did not include a proper configuration file because of the various possible testing setups. - I removed workarounds for IE since Angular itself does no longer support IE. Should these workarounds still be
kept?
-
TunnelService~uploadToStream
-
TunnelService~downloadStream
-
UserCredentials.getLink
-
UserCredentialService~getLink
-
- To replace the
$parse
function of AngularJS I used the NPM package angular-expressions which is a copy of the AngularJS code as standalone module. - There are some more questions directly in the code marked as a TODO.
Running the code
For my setup the easiest way to run the code was to build the frontend with maven and use the war file in a docker-compose setup. Since the module federation config is hard coded for now there will be some errors when the example extension (doc/guacamole-frontend-extension-example
) is not running (ng serve
) at localhost:4202
. If the extension is running, the content will be available at /extension-example.
It is also possible to run the frontend using ng serve
and benefit from live reloading with a way to work around the same origin policy (modify HTTP headers/proxy/disable web security). Therefore, the libraries need to be built manually: ng build guacamole-frontend-lib
and ng build guacamole-frontend-ext-lib
. Additionally, the following code might be necessary to redirect the requests to the REST API:
// projects/guacamole-frontend/src/app/auth/interceptor/authentication.interceptor.ts
if(!request.url.startsWith('assets'))
request = request.clone({url: `http://localhost:8080/guacamole/${request.url}`});
// projects/guacamole-frontend/src/app/client/services/managed-client.service.ts getInstance()
tunnel = new Guacamole.ChainedTunnel(
new Guacamole.WebSocketTunnel('ws://localhost:8080/guacamole/websocket-tunnel'),
new Guacamole.HTTPTunnel('http://localhost:8080/guacamole/tunnel')
When using ng serve
, the REST API from this branch should still be used.
To run the Cypress test I used the following configuration to disable web security for Chrome:
// guacamole/src/main/guacamole-frontend/cypress.config.ts
e2e: {
setupNodeEvents(on, config) {
on('before:browser:launch', (browser, launchOptions) => {
if (browser.family === 'chromium') {
launchOptions.args.push('--disable-web-security');
}
return launchOptions;
})
},
baseUrl: "http://localhost:4200"
}
:open_mouth:
Thanks, @leonard2901!
Are there any plans to release an official NPM package for guacamole-common-js?
If it should be done or needs to be done, then yes - absolutely.
Are there any plans to release an official NPM package for guacamole-common-js?
If it should be done or needs to be done, then yes - absolutely.
That would be great. Then it would be possible to also publish the Angular libraries to NPM and further simplify the creation of extensions and other applications.
Any feedback is appreciated - not limited to Mike of course. If anyone gave it a try (build/use/look-into), please post your comments.
I tried using ng serve
to deploy the webapp but it seems to have problems loading a module. Is this expected?
I deployed the app at /guacamole
/ and most things seem to run well, with a few exceptions (e.g. share link dropdown is broken, broken translations on text input buttons).
Super cool!
I tried using
ng serve
to deploy the webapp but it seems to have problems loading a module. Is this expected?
Thanks for taking the time to look at this! From the screenshot it looks like there are two problems with your setup:
- The Angular app can't find the example extension (
doc/guacamole-frontend-extension-example
) which is expected to be running on port 4202. This is currently hard-coded to show how extensions can be loaded in Angular without having to touch the Java code of the server. - If you run the app with
ng serve
, all requests will be made againstlocalhost:4200
by default. Please see the last section of the PR description for a workaround for this.
Hi! Thanks for your feedback so far. I’m working on finishing the remaining todos, and I wanted to ask if, once they’re done, it’s realistic to aim for a merge. Appreciate your thoughts!
I think so. It's quite a huge change, to be sure, and all the more reason to avoid too much long-term divergence.
Moving to Angular from AngularJS will mean a full version bump to 2.0.0, but that's OK. We can figure out a branching scheme that won't make maintenance of 1.5.x, upcoming 1.6.0, etc. impossible.
That sounds great.
Currently guacamole-common-js is provided here as an npm package. Type declarations for TypeScript are provided here.
With the transition to Angular and TypeScript, it would be nice if guacamole-common-js was offered as an "official" package at npm. @mike-jumper was already positive about this idea. What could be the procedure for realizing this?
With the transition to Angular and TypeScript, it would be nice if guacamole-common-js was offered as an "official" package at npm. @mike-jumper was already positive about this idea.
Ah, yes - that guy. ;)
What could be the procedure for realizing this?
The build would need to be altered however necessary to produce that package and consume it.
The current Maven-based process is:
-
The
guacamole-common-js
Maven project produces a.zip
artifact consumable by other Maven projects. -
The
guacamole
Maven project consumes theguacamole-common-js
artifact as part of the overallguacamole-client
build. -
When a release candidate is being produced, the
guacamole-common-js
artifact is pushed to a staging Maven repository.NOTE: It's important here that the artifact for a release candidate is the same as the artifact for the actual release. Its version number doesn't change, the binary doesn't get rebuilt; it gets staged, tested, and when the release passes vote its promoted directly without any modification/regeneration/rebuilding of any kind.
-
When a release candidate is being promoted to release, all staged artifacts (including
guacamole-common-js
) are released and get automatically synced to Maven Central.
An NPM-based process would need to be:
-
The
guacamole-common-js
Maven project produces an NPM artifact consumable by other NPM projects (and Maven projects that use NPM via plugins). If possible, we should also still produce a Maven artifact. -
The
guacamole
Maven project consumes theguacamole-common-js
artifact as part of the overallguacamole-client
build, whether via an NPM artifact or a pure Maven artifact. -
When a release candidate is being produced, the
guacamole-common-js
artifact is staged to NPM (somehow) as a release candidate. If we still produce a Maven artifact, that would be pushed to staging, too. As with the pure Maven process, it would be important that the staged artifact(s) are always the artifact(s) that are released - the release of a release candidate shouldn't involve rebuilding and potentially producing something that isn't what was tested as the release candidate. -
When a release candidate is being promoted to release, all staged artifacts (including
guacamole-common-js
) are released and get automatically synced to Maven Central, NPM, etc.
In addition to that: Perhaps it would be a good idea to reserve a good name as 'guacamole' as package seems to be taken https://www.npmjs.com/package/guacamole There is an organization level naming schema as well, so "@guacamole" or "@apache-guacamole" might be useful to save for this use case.
@everflux Sadly, it looks like the guacamole
name is already taken at the org level, so apache-guacamole
or a similar variant might be all that's possible (unless the owner of that org/username is willing to relinquish it):
https://www.npmjs.com/org/guacamole
As for the package itself, I think a reasonable path forward might be to get in contact with @padarom, the individual that produced and maintains an NPM-ified fork of guacamole-common-js:
https://www.npmjs.com/package/guacamole-common-js
If we can take over ownership and maintenance of that package within NPM, then existing users of the established NPM package would be able to continue using it without interruption, and former users of the original pure-Maven package looking to migrate to NPM would have the benefit of familiar package naming.
@padarom, any thoughts?
The intention of my fork was to make it easier for people to use Guacamole in their JS projects, so if there ends up being an official Apache package doing the same, then my fork can be seen as obsolete.
As long as the official npm package is compatible, I'm fine with transferring ownership. If I understood correctly the plan seems to include a major version bump, so even if it isn't quite compatible some breakage could be expected by the package's current user base anyways.
GitHub's diff viewer isn't being nice to me with this large PR and I'm currently on the go, but from the PR description it seems like the regular Guacamole frontend is rewritten in Typescript, whereas guacamole-common-js
is not. Are type definitions still expected to be provided via @types/guacamole-common-js
? If I could make the recommendation to also think about including (and bundling) types directly in the official package, rather than having users manually add another package that has to be separately maintained?
If I could make the recommendation to also think about including (and bundling) types directly in the official package, rather than having users manually add another package that has to be separately maintained?
I agree.
If I understood correctly the plan seems to include a major version bump, so even if it isn't quite compatible some breakage could be expected by the package's current user base anyways.
I see no immediate reason why my changes to the frontend should lead to breaking changes in the JS lib.
If I could make the recommendation to also think about including (and bundling) types directly in the official package, rather than having users manually add another package that has to be separately maintained?
I also agree. However, to avoid making this PR even bigger, I would suggest that this should be done as a separate task after this PR has been merged
@leonard2901: when you are planning to release your guacamole-client-angular as a npm package?
@suncase Thank you for you interest in the Angular client. As you can see in the description of the PR, there are still a few things I need to work on before this could possibly be merged. Unfortunately, I currently have less time for the project than I would have liked. So I can't tell you when this will be merged.
That being said, I don't think the frontend of the Guacamole Client will be released as an npm package. My idea is to publish the packages "guacamole-frontend-ext-lib" and "guacamole-frontend-lib" on npm to facilitate 1) the creation of extensions for the frontend and 2) the creation of custom Guacamole-Angular applications. If I have overlooked a use case, please feel free to describe it.