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

Update angular and ukis #71

Closed
MichaelLangbein opened this issue Jan 24, 2022 · 7 comments
Closed

Update angular and ukis #71

MichaelLangbein opened this issue Jan 24, 2022 · 7 comments

Comments

@MichaelLangbein
Copy link
Collaborator

There is a bug where selecting another earthquake does not cause the 'selected-eq' layer to change.

That bug is caused by an older version (7) of ukis. Updating ukis to a newer version would solve the issue.

Updating is not quite that easy, however.

  • Ukis 8 requires at least angular 12, whereas currently we're using angular 10.
  • Angular 12 is incompatible with our current version of ngrx (10).
  • The newer version of ngrx has a new syntax and stricter type-checking, requiring us to re-write some portion of our state-management.

In other words, fixing this bug might require a rather complicated update of ukis, angular and ngrx.

@MichaelLangbein
Copy link
Collaborator Author

One might want to consider that long term we need to do some restructuring to the code, anyway.

In the long term processes and products will no longer be hardcoded into the frontend, but instead be red from a database. This requires us to separate the processing- and the render-logic in our current Process-classes. That's a model that is more compatible with how ngrx sees the world. We might be able to do some work in that direction during this update.

@MichaelLangbein
Copy link
Collaborator Author

There is a nice upgrade-guide for ngrx here: https://v11.ngrx.io/guide/migration/v11

@MichaelLangbein
Copy link
Collaborator Author

Lots of dependency conflicts. Listing them here so I can keep track of them all.

Following https://update.angular.io/?l=3&v=10.1-12.0 and https://ngrx.io/guide/migration/v12

Issue 1: on npx @angular/cli@11 update @angular/core@11 @angular/cli@11

npm ERR! While resolving: riesgos@0.0.0
npm ERR! Found: @angular-devkit/build-angular@0.1000.8
npm ERR! node_modules/@angular-devkit/build-angular
npm ERR!   dev @angular-devkit/build-angular@"~0.1102.18" from the root project
npm ERR!
npm ERR! Could not resolve dependency:
npm ERR! dev @angular-devkit/build-angular@"~0.1102.18" from the root project
npm ERR!
npm ERR! Conflicting peer dependency: @angular/compiler-cli@11.2.14
npm ERR! node_modules/@angular/compiler-cli
npm ERR!   peer @angular/compiler-cli@"^11.0.0 || ^11.2.0-next" from @angular-devkit/build-angular@0.1102.18
npm ERR!   node_modules/@angular-devkit/build-angular
npm ERR!     dev @angular-devkit/build-angular@"~0.1102.18" from the root project

Issue caused by a known bug in npm 7+. Downgrading to npm v6 resolves the problem.

@MichaelLangbein
Copy link
Collaborator Author

MichaelLangbein commented Feb 18, 2022

Issue 2: running after upgrading to angular 11:

Error: node_modules/@clr/angular/accordion/stepper/stepper-panel.d.ts:18:9 - error TS2611: 'id' is defined as a property in class 'ClrAccordionPanel', but is overridden here in 'ClrStepperPanel' as an accessor.

18     get id(): string;

Related to this issue in clarity; resolved by updating clarity to version 4.0.16

@MichaelLangbein
Copy link
Collaborator Author

MichaelLangbein commented Feb 19, 2022

On angular 11, attempting to update to angular 12.
Issue 3: npx @angular/cli@12 update @angular/core@12 @angular/cli@12

Using package manager: 'npm'
Collecting installed dependencies...
Found 76 dependencies.
Fetching dependency metadata from registry...
                  Package "@clr/angular" has an incompatible peer dependency to "@angular/common" (requires "^10.0.0" (extended), would install "12.2.16").
                  Package "@clr/angular" has an incompatible peer dependency to "@angular/core" (requires "^10.0.0" (extended), would install "12.2.16").
                  Package "@dlr-eoc/core-ui" has an incompatible peer dependency to "@angular/router" (requires "~10.0.14" (extended), would install "12.2.16").
× Migration failed: Incompatible peer dependencies found.
Peer dependency warnings when installing dependencies means that those dependencies might not work correctly together.
You can use the '--force' option to ignore incompatible peer dependencies and instead address these warnings later.   
  See "C:\Users\lang_m13\AppData\Local\Temp\ng-E6OWj1\angular-errors.log" for further details.

Clearly, the version of clarity I have installed cannot handle the new angular libraries. That's ok, we'll update it once angular is updated.

npx @angular/cli@12 update @angular/core@12 @angular/cli@12 --force
Remarkably, this added the following entries to angular.json:

            "vendorChunk": true,
            "extractLicenses": false,
            "buildOptimizer": false,
            "sourceMap": true,
            "optimization": false,
            "namedChunks": true

We might be able to make use of extractLicenses, buildOptimizer and optimization later.

Either way, we are now in a state where we have angular12 but still clarity4 and ukis7. These versions are probably incompatible.

npx ng update @clr/angular@^12 --force

npx ng update @ngrx/store@12

npm install @dlr/eoc/*@^8

Renamed all occurances of pupupFunktion to popupFunction.

@MichaelLangbein
Copy link
Collaborator Author

MichaelLangbein commented Feb 19, 2022

Issue 4: Trying to run now causes:

./node_modules/amdefine/amdefine.js:26:15-30 - Error: Module not found: Error: Can't resolve 'path' in 'C:\Users\lang_m13\Desktop\code\js\ukis_projects\riesgos\dlr-riesgos-frontend\node_modules\amdefine'

BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:
        - add a fallback 'resolve.fallback: { "path": require.resolve("path-browserify") }'
        - install 'path-browserify'
If you don't want to include a polyfill, you can use an empty module like this:
        resolve.fallback: { "path": false }

amdefine is a dependency of jsonix. This is a problem - jsonix is no longer maintained. But: I do maintain my own fork of the library. Might be able to fix the issue there.
Ironically, amdefine is a node-translation of the browser-centric amd. We are now in a browser-app, installing a node-shim, so that we can use that node-shim to simulate a browser-dependency-resolver.
Maybe we can extract the raw js from the js/java repo jsonix, and create our own pure-js version

For now I've ...

  • copied the wps-code from ukis to riesgos for faster code-turnaround
  • copied all jsonix-js code from the jsonix package directly into the wps-code

@MichaelLangbein
Copy link
Collaborator Author

MichaelLangbein commented Feb 20, 2022

Issue 5: After upgrade to angular 12, a bunch of type errors in our libraries are detected.

Error: node_modules/@dlr-eoc/map-ol/node_modules/ol/ImageBase.d.ts:32:48 - error TS2694: Namespace '"C:/Users/lang_m13/Desktop/code/js/ukis_projects/riesgos/dlr-riesgos-frontend/node_modules/@dlr-eoc/map-ol/node_modules/ol/ImageState"' has no exported member 'default'.

32     protected state: import("./ImageState.js").default;
                                                  ~~~~~~~


Error: node_modules/@dlr-eoc/map-ol/node_modules/ol/Tile.d.ts:131:47 - error TS2694: Namespace '"C:/Users/lang_m13/Desktop/code/js/ukis_projects/riesgos/dlr-riesgos-frontend/node_modules/@dlr-eoc/map-ol/node_modules/ol/TileState"' has no exported member 'default'.

131     protected state: import("./TileState.js").default;
                                                  ~~~~~~~


Error: node_modules/@dlr-eoc/map-ol/node_modules/ol/geom/SimpleGeometry.d.ts:27:53 - error TS2694: Namespace '"C:/Users/lang_m13/Desktop/code/js/ukis_projects/riesgos/dlr-riesgos-frontend/node_modules/@dlr-eoc/map-ol/node_modules/ol/geom/GeometryLayout"' has no exported member 'default'.

27     protected layout: import("./GeometryLayout.js").default;
                                                       ~~~~~~~


Error: node_modules/@dlr-eoc/map-ol/node_modules/ol/interaction/DragBox.d.ts:25:22 - error TS2314: Generic type 'MapBrowserEvent<EVENT>' requires 1 type argument(s).

25     mapBrowserEvent: import("../MapBrowserEvent.js").default;
                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


Error: node_modules/@dlr-eoc/map-ol/node_modules/ol/interaction/Draw.d.ts:39:14 - error TS2314: Generic type 'Feature<Geometry>' requires 1 type argument(s).

39     feature: Feature;
                ~~~~~~~


Error: node_modules/@dlr-eoc/map-ol/node_modules/ol/interaction/Modify.d.ts:77:22 - error TS2314: Generic type 'MapBrowserEvent<EVENT>' requires 1 type argument(s).

77     mapBrowserEvent: import("../MapBrowserEvent.js").default;
                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


Error: node_modules/@dlr-eoc/map-ol/node_modules/ol/interaction/Modify.d.ts:330:30 - error TS2314: Generic type 'BaseVectorLayer<VectorSourceType>' requires 1 type argument(s).

330     hitDetection_: boolean | import("../layer/BaseVector").default;
                                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


Error: node_modules/@dlr-eoc/map-ol/node_modules/ol/interaction/Select.d.ts:82:21 - error TS2314: Generic type 'Feature<Geometry>' requires 1 type argument(s).

82     selected: Array<import("../Feature.js").default>;
                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


Error: node_modules/@dlr-eoc/map-ol/node_modules/ol/interaction/Select.d.ts:88:23 - error TS2314: Generic type 'Feature<Geometry>' requires 1 type argument(s).

88     deselected: Array<import("../Feature.js").default>;
                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


Error: node_modules/@dlr-eoc/map-ol/node_modules/ol/interaction/Select.d.ts:94:22 - error TS2314: Generic type 'MapBrowserEvent<EVENT>' requires 1 type argument(s).

94     mapBrowserEvent: import("../MapBrowserEvent.js").default;
                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


Error: node_modules/@dlr-eoc/map-ol/node_modules/ol/interaction/Translate.d.ts:47:26 - error TS2314: Generic type 'Feature<Geometry>' requires 1 type argument(s).

47     features: Collection<import("../Feature.js").default>;
                            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


Error: node_modules/@dlr-eoc/map-ol/node_modules/ol/interaction/Translate.d.ts:67:22 - error TS2314: Generic type 'MapBrowserEvent<EVENT>' requires 1 type argument(s).

67     mapBrowserEvent: import("../MapBrowserEvent.js").default;
                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


Error: node_modules/@dlr-eoc/utils-maps/node_modules/ol/source/Cluster.d.ts:136:31 - error TS2314: Generic type 'Feature<Geometry>' requires 1 type argument(s).

136     protected features: Array<Feature>;
                                  ~~~~~~~


Error: node_modules/@dlr-eoc/utils-maps/node_modules/ol/source/Cluster.d.ts:152:23 - error TS2314: Generic type 'VectorSource<Geometry>' requires 1 type argument(s).

152     protected source: VectorSource;
                          ~~~~~~~~~~~~

This is an issue with @dlr-eoc (will create an issue in that repo later) which we cannot really handle in this project. Instead just setting "skipLibCheck" in tsconf. But in the future we should better revert that setting (for increased type-security).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant