Upgrading to angular 14

lacroixdavid1 commented 2 years ago

This one is similar to https://github.com/FERNman/angular-google-charts/pull/254 but it bumps every lib. It also allows the test to run. Some of them are failing due to spies to getElementsByTagNames not satisfying Angular TestBed resetTestingModule trying to call the same method. Even empty spec throw when spying on this method.

 TypeError: Cannot convert undefined or null to object
        at slice (<anonymous>)

      at byTag (../../node_modules/nwsapi/src/nwsapi.js:398:24)
      at Array.<anonymous> (../../node_modules/nwsapi/src/nwsapi.js:348:117)
      at Object._querySelectorAll [as select] (../../node_modules/nwsapi/src/nwsapi.js:1475:36)
      at DOMTestComponentRenderer.removeAllRootElements (../../node_modules/@angular/platform-browser-dynamic/fesm2015/testing.mjs:37:36)
      at TestBedRender3.tearDownTestingModule (../../node_modules/@angular/core/fesm2015/testing.mjs:1885:95)
      at TestBedRender3.resetTestingModule (../../node_modules/@angular/core/fesm2015/testing.mjs:1682:26)
      at Function.resetTestingModule (../../node_modules/@angular/core/fesm2015/testing.mjs:1620:30)

I'm not familiar with Jest that much, so I'm not able to fix it. I was able to start the test application using npm run start and I had no errors.

Could we please have this one merge?

Feel free to use my fork until we have this merged by having this line in your packages.json.

    "angular-google-charts": "git://github.com/lacroixdavid1/angular-google-charts.git#ng14-dist",

baptistedec commented 2 years ago

Any update on this?

realsteel1 commented 2 years ago

Hi all, any further updates on this one? Thank you :-)

urosTeo commented 2 years ago

Any updates on this one? :smile:

vogloblinsky commented 2 years ago

Please, is it possible to have an update on this, or for ng 14 just released ?

lacroixdavid1 commented 2 years ago

@FERNman mind helping me with the failing test?

oobayly commented 2 years ago

@FERNman mind helping me with the failing test?

Hi @lacroixdavid1, it looks like you haven't updated the peer dependencies in libs/angular-google-charts/package.json to 14.x.x

"peerDependencies": {
    "@angular/common": ">=6.0.0 <=14.x.x",
    "@angular/core": ">=6.0.0 <=14.x.x"
  },

After I did that, I was able to run npm -i and ng build successfully.

lacroixdavid1 commented 2 years ago

@FERNman mind helping me with the failing test?

Hi @lacroixdavid1, it looks like you haven't updated the peer dependencies in libs/angular-google-charts/package.json to 14.x.x
"peerDependencies": {
    "@angular/common": ">=6.0.0 <=14.x.x",
    "@angular/core": ">=6.0.0 <=14.x.x"
  },
After I did that, I was able to run npm -i and ng build successfully.

Were you able to run the unit test and have them all succeed?

oobayly commented 2 years ago

@lacroixdavid1 :facepalm: I misunderstood your comment as getting the npm install portion of circleci to succeed - sorry.

oobayly commented 2 years ago

Ok, so I had another look at this. The trick appears to ensure that the spys are restores after each test. I've got the tests to run (and pass), but a couple of them look like they should be tidied up.

I can post a diff if you'd like.

hrueger commented 2 years ago

I fixed the peerDependencies in my fork of the fork:

npm i git://github.com/hrueger/angular-google-charts.git#ng14-dist

and created a PR to your fork @lacroixdavid1

oobayly commented 2 years ago

@hrueger Did you get the tests to run once updating the peer dependencies? I couldn't, and had to do the following:

While trying to work out why I had to update tests for them to pass, even though it really didn't seem like it should be necessary, I delved a bit deeper.

Updating the peerDependencies does allow the npm install to execute sucessfully, but...
- the tests don't run.
- npm update will fail with dependency conflicts due to app/playground still targeting Angular 13

So, the next step is to update app/playground/package.json to taget Angular 14. To make life easier I just ran:

npm-check-updates -u --deep to ensure all the packages are targeting the same versions.
Note: This resets the peerDependencies in /lib/angular-google-charts/package.json so they need to be reset to back to ">=6.0.0 <=14.x.x"
Followed by npm update

After I'd done that I managed to get build, test and start to run sucessfully without faffing around with tests (which just felt wrong).

oobayly commented 2 years ago

@lacroixdavid1 I hope you don't mind, but I'd quite like to get this in a position to merge, so I created a 2nd PR (#278) in order to verify that the tests I managed to get working locally would pass on CircleCI.

It uses the changes that you made to get the build to work, but also bump and consolidates all the package versions as I described above.

Another issue turned out to be that the the node version on CircleCI needed to be bumped to 16.10, and also the coverage flag for tests needed to be changed.

I'm happy to close my PR and create a PR for your fork seeing as you did the ground work.

hrueger commented 2 years ago

Hi @oobayly, Great to see that you're working on this. Of course I don't mind, I just needed something to quickly fix my build as I had to release it earlier today and that was breaking my build. Then, I thought I'd just create a PR, but yours looks way more complete. So I'll just close mine, no problem 👍

lacroixdavid1 commented 2 years ago

Closing in favor of https://github.com/FERNman/angular-google-charts/pull/278

FERNman / angular-google-charts

Upgrading to angular 14 #271