Angular upgrade v18 #487

gsambrotta · 2024-08-13T12:16:13Z

Here is the upgrade to angular 18.
Issue ref: #476 - In the issue, it can be found a good list of the commands used to upgrade.

HOW TO TEST IT:

You will need node v.18+ to test this branch
after checking this branch out, remember to update node packages npm i
If you get any error of incompatible version or packages to be updated, I would suggest clearing the cache of NPM packages or even deleting package-lock.json (last resource), if errors persist.
You can check the preview of this branch here: https://kendraio-app-git-angular-upgrade-18-kendraio.vercel.app/
Below you can see the main visual difference between v.16 and v.18 Everything else should be the same.

What is missing:

Fix Ag Grid / Issue with custom components
Fix small details of UI
Fix/Double check Forms
Leaflet issue
Material UI not apply to widget Array Type

See screenshots:
AgGrid do not display custom components and do not show any error
v18

v15

Material UI padding is slightly wrong:
v18

v15

Forms (or material UI?)
v18

v15

Leaflet issue:
v18

Widget Form Issue:
v18

vercel · 2024-08-13T12:16:17Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
kendraio-app	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Sep 27, 2024 3:40pm

gsambrotta · 2024-08-22T13:21:55Z

This is a replica of our app configuration to test and work on the Map issue: https://codesandbox.io/p/devbox/gracious-tesla-4ts9k4

The error persist also in a mock up environment.
It seems to be related to the way we initiate markerClusterData: [];
It might be that, initiate with an empty array is not allowed any more.

…ll broken

…ic file from parent package

…re fix, removes ngModelGroup. Materia-ui module was stale. Prior setDiagnosticOptions configuration caused error due to missing object. The ngModelGroup template property also caused runtime error and is not needed.

…span child element covering is acceptable.

…ting! Needs fixing later!

…nfirm test

* restore correct UI style to grid * apply breaking changes with frmeworkComponents, bump version to 31

gsambrotta · 2024-08-29T13:14:54Z

This PR is ready to be reviewed.
The only issue still open is the one regarding the Widget type array without style. We agreed to open a separate Issue for that problem, not to block this big PR.
Array type issue: #491

CodeKrakken

Phew! Good work. The code looks good, aside from a few stray merge markers.

I had to upgrade node.js to install and test these code changes. Not a problem obviously, but it would be neater if any required upgrades were done under the hood. Can we add the minimum required node version as a dependency in package.json?

docs/workflow/blocks/launch_block.rst

docs/workflow/implementation.rst

docs/workflow/intro.rst

gsambrotta · 2024-09-11T11:56:13Z

Hey @CodeKrakken thank you very much for your review :D

About node version:

I had to upgrade node.js to install and test these code changes. Not a problem obviously, but it would be neater if any required upgrades were done under the hood. Can we add the minimum required node version as a dependency in package.json?

In the package.json there is already specified the minimum required node version

"engines": {
    "node": ">=18.19.0"
  },

We cannot really enforce it. We can add a .npmrc to not allow installing the project if the node version is not correct, or we can add a
.nvmrc to help users who use nvm.
Do you have any other ideas how we can do it?

Also, which node version you had before and which one you have installed now?
And which error you got it that made you realize the node version was incompatible?
I am curious to understand for future cases, thank you!

CodeKrakken · 2024-09-11T13:17:09Z

@gsambrotta I think it was 17.6.0. I've rolled back to recreate the errors but it's not the same this time around.

The first issue was when I ran ng test - it threw various errors and couldn't find the tests. Of course then I realised I hadn't yet run npm install. Now I've rolled back it's just telling me politely that The Angular CLI requires a minimum of v18.13.

npm install is giving me more or less the same as yesterday - it's clear what the problem is.

error.txt

I don't have any ideas about how to enforce the node version unfortunately. It's a shame we can't add it as a dependency!

gsambrotta · 2024-09-17T09:06:18Z

@CodeKrakken thank you for clarifying what had happen.
This Angular version has some major change and it will definitely need node, angular cli to be updated.
I should write it in the PR description, thanks.

Theoretically they should update by their self when run npm i. I wonder if even the angular cli update. I wonder what happen if somoene has installed the Angular CLI globally. I guess it should still download a project-scoped version and upgrade that version.

I also woner if we should suggest to run npm i or rather ng update. But I think npm i it should cover everything.

CodeKrakken

Just one bit of possibly stray text.

docs/workflow/blocks/launch_block.rst

lukestanley

I do wish that the Workflow to Flow changes and other unrelated changes were not in this branch, OR instead, if 5645a0a could be merged into develop and main that would be better.
There are some whitespace changes that don't make sense to me.

I have a few other questions I commented on.

Anyway, these are mostly small things. In general, this has been a lot of work that you've boldly wrestled with, well done!

Once the smaller comments are understood and resolved, I am thinking that since it's so big and complex and because we can't know what the Angular reactivity changes will be, and I don't trust the testing of the Vercel environment to be as comprehensive as what real world use of the Flows on the production version will be, it might make sense for us at some point soon to try it for a while in production for real, and see what kinds of regressions there are, of course we can always revert back.

Well done! Thanks for your diligent and patient work!

lukestanley · 2024-09-19T13:42:29Z

src/app/_shared/404.interceptor.ts

-import {
-    HttpInterceptor,
-    HttpRequest,
-    HttpResponse,
-    HttpHandler,
-    HttpEvent,
-    HttpErrorResponse
-} from '@angular/common/http';
+import { HttpInterceptor, HttpRequest, HttpResponse, HttpHandler, HttpEvent, HttpErrorResponse } from '@angular/common/http';


Do you know why this changed?

Nope, I didn't change, not in purpose at least, as I prefer the older formatting style
I think that it is better to take care of the formatting issues once we have a formatting style guide in place.

It happened in e6862e1which is a commit under your name, it appears, maybe it was automatically done somehow, perhaps as part of a migration script?
It does make me wonder if we should force linting on all branches before we apply this. 🤔

src/app/_shared/services/uphold_service.ts

src/app/_shared/ui-form/types/password.type.ts

src/app/app.module.ts

lukestanley · 2024-09-19T13:48:53Z

src/app/blocks/grid-block/grid-block.component.ts

@@ -1,6 +1,5 @@
 import { Component, ElementRef, EventEmitter, Input, NgZone, OnChanges, OnInit, Output, ViewChild } from '@angular/core';
-import { clone, get, has, isArray, isObject, toPairs } from 'lodash-es';
-import { search } from 'jmespath';


Presumably search wasn't used?

What is your question exactly?
I deleted these imports because they were highlighted as imported but never used in the document.

It wasn't clear why it was deleted, and you've said now.
But in future, please can we keep the difference as small as possible and do tidying separately so it's easier to review, as a general rule?

lukestanley · 2024-09-19T13:53:04Z

src/app/components/workflow-sidenav/workflow-sidenav.component.html

+        <p class="adapter-metadata-edit-text">
+          Title: {{ workflow.title }}<br>
+          Adapter: {{ workflow.getAdapterName() }}<br>
+          Workflow ID: {{ workflow.id }}<br>


This may be a regression, since if said Flow before?

Where do you see that it was saying Flow before? If you look at the Git Diff, you can see that the {{workflow.title}} and similar are the same.
What has changed here is the class of the DIV and P tag
@lukestanley

On the existing version line 49 said "Flow ID". On line 45 in the new version it says "Workflow ID".

src/app/dialogs/edit-workflow-metadata-dialog/edit-workflow-metadata-dialog.component.ts

src/app/directives/track-clip.directive.ts

lukestanley · 2024-09-19T13:56:38Z

src/app/pages/blocks-builder-page/blocks-builder-page.component.ts

-import {AdapterBlocksConfigSelectDialogComponent} from
-    '../../dialogs/adapter-blocks-config-select-dialog/adapter-blocks-config-select-dialog.component';
-import {ExportConfigDialogComponent} from '../../dialogs/export-config-dialog/export-config-dialog.component';
-import {PasteConfigDialogComponent} from '../../dialogs/paste-config-dialog/paste-config-dialog.component';
-import * as stringify from 'json-stringify-safe';


What happened here?

All of these were "declared but their value is never read"
So i cleaned up a bit

lukestanley · 2024-09-19T13:57:39Z

src/app/services/adapter-install.service.ts

 import {AppSettingsService} from './app-settings.service';
 import { MatSnackBar } from '@angular/material/snack-bar';
-import * as LZS from 'lz-string';


I suppose LSZ is not used here now?

It was never used. It was an import not used in the file

lukestanley · 2024-09-19T14:09:11Z

For reference, I used this view (and some variations) to try and compare the changes you made @gsambrotta

dahacouk · 2024-09-19T15:18:50Z

Hi @gsambrotta and @lukestanley

I do wish that the Workflow to Flow changes and other unrelated changes were not in this branch, OR instead, if 5645a0a could be merged into develop and main that would be better.

I'm curious how this situation came about in the first place. Perhaps we could discuss sometime?

Great work Gio!

gsambrotta · 2024-09-27T15:35:28Z

Hi @gsambrotta and @lukestanley

I do wish that the Workflow to Flow changes and other unrelated changes were not in this branch, OR instead, if 5645a0a could be merged into develop and main that would be better.

I'm curious how this situation came about in the first place. Perhaps we could discuss sometime?

Great work Gio!

I am curious too.
I believe this commit 5645a0a is already merged, probably both in develop and main but for sure in develop

I think all these old commits are parts of the history of this branch, as it is so old but the changes are not visible, since it has no difference with develop. Which is the branch we want to merge into
Or what did you mean exactly with that comments, @lukestanley ?

CodeKrakken

All comments addressed, all tests passing. Nice job!

lukestanley · 2024-10-04T13:07:41Z

Hi @gsambrotta and @lukestanley

I do wish that the Workflow to Flow changes and other unrelated changes were not in this branch, OR instead, if 5645a0a could be merged into develop and main that would be better.

I'm curious how this situation came about in the first place. Perhaps we could discuss sometime?
Great work Gio!

I am curious too. I believe this commit 5645a0a is already merged, probably both in develop and main but for sure in develop

I think all these old commits are parts of the history of this branch, as it is so old but the changes are not visible, since it has no difference with develop. Which is the branch we want to merge into Or what did you mean exactly with that comments, @lukestanley ?
Unfortunately, develop is NOT updated with the Workflow to Flow changes.
We need to untangle this by getting develop to have the changes.
I have commented on the existing Pull Request:
#451 (comment)

Once that Workflow to Flow pull request is resolved, I suspect that this Pull Request will be in a much better shape!

By the way, I'm not sure fixing comments from review as a commit message would help inform people much!
@gsambrotta

lukestanley · 2024-11-28T11:59:59Z

The very valuable work from this was put into Angular 18 Squashed, which is now merged into develop, so I am closing this. It could still serve as very useful reference.

vercel bot deployed to Preview August 14, 2024 18:57 View deployment

gsambrotta force-pushed the angular-upgrade-18 branch from cf6d7df to df49f59 Compare August 27, 2024 12:12

vercel bot had a problem deploying to Preview August 27, 2024 12:13 Failure

vercel bot deployed to Preview August 28, 2024 13:28 View deployment

vercel bot deployed to Preview August 29, 2024 12:57 View deployment

lukestanley and others added 23 commits August 29, 2024 14:58

Update TypeScript version and add ng-update

f97475e

Updates Angular dependencies and remove unused CanDeactivate import

2e295b6

Angular 16 update: up eslint

01e63fb

Angular 16 update: up cdk

b4bfe34

Angular 16 update: update Material library

d1e51f3

Angular 16 Upgrade: Get frontend working, flow and query builders sti…

ee3194c

…ll broken

Angular 16: Remove legacy materials

2156c59

Fixes Monaco-editor use by using v2 wrapper with correct path to stat…

ee35655

…ic file from parent package

Fixes Cypress tests with settings button click - enable force click, …

5cc936a

…span child element covering is acceptable.

Cypress gosub bug fix (changed selector)

9a4debd

Prevent Cypress warning by naming mock

4bbc392

Force Cypress to click "hidden" areas

dc41123

FIXME: Ignores REAL error of MatFormField placeholder text not popula…

f028ef7

…ting! Needs fixing later!

Names intercept mock to avoid Cypress warning

e885a08

Clarifies that a mock is used

c188651

Fix: Renames old Formly validation property

18f8f13

Allows Lodash CommonJS property tidy console warnings

befce71

Work around reactivity problem by pressing button and tries fixing co…

9675d11

…nfirm test

Disables broken confirm test

9b7f6c5

Update zone.js and adds back package lock

65550f7

Reverting automated prettier changes to reduce non-needed changes

efa5e13

Fix Ag Grid UI and custom component render (#447)

0c4bec8

* restore correct UI style to grid * apply breaking changes with frmeworkComponents, bump version to 31

gsambrotta added 3 commits August 29, 2024 15:00

update leafleft component

cbd25a4

render map with marker

1107cf4

fix map issue

2c60e86

gsambrotta marked this pull request as ready for review August 29, 2024 13:05

gsambrotta requested review from lukestanley and CodeKrakken August 29, 2024 13:05

gsambrotta force-pushed the angular-upgrade-18 branch from 6c22869 to 2c60e86 Compare August 29, 2024 13:06

vercel bot deployed to Preview August 29, 2024 13:07 View deployment

gsambrotta mentioned this pull request Aug 29, 2024

Array Field type not styled after upgrade to Angular 18 #491

Open

fix error on test file

be10138

vercel bot deployed to Preview September 4, 2024 12:07 View deployment

CodeKrakken requested changes Sep 10, 2024

View reviewed changes

clean up merge conflicts

14fbbe1

vercel bot deployed to Preview September 11, 2024 12:02 View deployment

CodeKrakken requested changes Sep 17, 2024

View reviewed changes

docs/workflow/blocks/launch_block.rst Outdated Show resolved Hide resolved

lukestanley reviewed Sep 19, 2024

View reviewed changes

fixing comments from review

e67efd0

vercel bot deployed to Preview September 27, 2024 15:40 View deployment

CodeKrakken approved these changes Oct 2, 2024

View reviewed changes

lukestanley mentioned this pull request Nov 12, 2024

Angular 18 upgrade (squashed) #500

Merged

lukestanley closed this Nov 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Angular upgrade v18 #487

Angular upgrade v18 #487

gsambrotta commented Aug 13, 2024 •

edited

Loading

vercel bot commented Aug 13, 2024 •

edited

Loading

gsambrotta commented Aug 22, 2024 •

edited

Loading

gsambrotta commented Aug 29, 2024

CodeKrakken left a comment

gsambrotta commented Sep 11, 2024

CodeKrakken commented Sep 11, 2024

gsambrotta commented Sep 17, 2024

CodeKrakken left a comment

lukestanley left a comment

lukestanley Sep 19, 2024

gsambrotta Sep 24, 2024

lukestanley Oct 8, 2024

lukestanley Sep 19, 2024

gsambrotta Sep 24, 2024

lukestanley Oct 8, 2024

lukestanley Sep 19, 2024 •

edited

Loading

gsambrotta Sep 27, 2024

lukestanley Oct 8, 2024

lukestanley Sep 19, 2024

gsambrotta Sep 27, 2024

lukestanley Sep 19, 2024

gsambrotta Sep 24, 2024

lukestanley commented Sep 19, 2024

dahacouk commented Sep 19, 2024

gsambrotta commented Sep 27, 2024

CodeKrakken left a comment

lukestanley commented Oct 4, 2024

lukestanley commented Nov 28, 2024

Angular upgrade v18 #487

Angular upgrade v18 #487

Conversation

gsambrotta commented Aug 13, 2024 • edited Loading

vercel bot commented Aug 13, 2024 • edited Loading

gsambrotta commented Aug 22, 2024 • edited Loading

gsambrotta commented Aug 29, 2024

CodeKrakken left a comment

Choose a reason for hiding this comment

gsambrotta commented Sep 11, 2024

CodeKrakken commented Sep 11, 2024

gsambrotta commented Sep 17, 2024

CodeKrakken left a comment

Choose a reason for hiding this comment

lukestanley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lukestanley Sep 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lukestanley commented Sep 19, 2024

dahacouk commented Sep 19, 2024

gsambrotta commented Sep 27, 2024

CodeKrakken left a comment

Choose a reason for hiding this comment

lukestanley commented Oct 4, 2024

lukestanley commented Nov 28, 2024

gsambrotta commented Aug 13, 2024 •

edited

Loading

vercel bot commented Aug 13, 2024 •

edited

Loading

gsambrotta commented Aug 22, 2024 •

edited

Loading

lukestanley Sep 19, 2024 •

edited

Loading