Upgrade to version 2.1.0 breaks #111

minuz · 2018-03-26T01:07:44Z

Hi,

I was updating the packages from my solution and when upgrading from 2.0.3 to 2.1.0 of your library, all of sudden my screen stopped working.

I had a dynamic form that receives the framework type at the creation of the module and I'm using your library to abstract the dynamic injection of the component

private _collectChangesFromDiffer(differ: any): SimpleChanges {
    const changes = {} as SimpleChanges;

    // It's breaking here! It seems like the differ is null or undefined, causing the loop to break.
    differ.forEachItem((record: KeyValueChangeRecordAny) =>
      changes[record.key] = new CustomSimpleChange(record.previousValue, record.currentValue, false));

    differ.forEachAddedItem((record: KeyValueChangeRecordAny) =>
      changes[record.key].previousValue = UNINITIALIZED);

    return this._resolveChanges(changes);
  }

before (v.2.0.3) :

    if (inputsChanges) {
      const isNotFirstChange = !!this._lastInputChanges;
      this._lastInputChanges = this._collectChangesFromDiffer(inputsChanges);

      if (isNotFirstChange) {
        this.updateInputs();
      }
    }

Current:

    if (inputsChanges) {
      const isNotFirstChange = !!this._lastInputChanges;
      this._updateInputChanges(inputsChanges);

      if (isNotFirstChange) {
        this.updateInputs();
      }
    }

Can you please have a look? It seems the caller of this function changed and something within the new routine is not happy. Possibly due to the fact the _collectChangesFromDiffer() is called twice. One within the ngOnChanges and then on ngOnCheck.

The text was updated successfully, but these errors were encountered:

gund · 2018-03-26T03:01:03Z

Hi, that's weird I had green light across the board in tests...

Will take a look.

minuz · 2018-03-26T09:06:54Z

Hi @gund, thanks for the fast reply.
I can confirm that switching back to 2.0.3 fixes my current issue.
Please, let me know if you need more details from me.

gund · 2018-03-26T11:55:24Z

Hey @minuz, I just tried simple demo with latest (v2.1.0) version of this lib and it seems to work fine:

No errors and changes are computed as expected...

You can check it out here: https://stackblitz.com/edit/angular-qywrkv

Maybe you can share example where it crashes?

UPDATE: Sorry I updated the link, now -> https://stackblitz.com/edit/test-ng-dynamic-component

st-clair-clarke · 2018-03-26T20:25:35Z

I agree with @minuz. My application is broken as well with the new upgrade. Here is the error from my app

Note the _collectChangesFromDiffer present in the error and in @minuz.

Also, can we have a change-log please.

Cheers

gund · 2018-03-26T21:50:11Z

Can any of you please provide minimal repro example either on plunkr or stakblitz so I can have a look?

Because with the simple example it works just fine...

minuz · 2018-03-26T22:40:15Z

Hi @gund, I'll try to reproduce that in isolation. I'll let you know to see if I could reproduce the issue with minimum steps.

jzoss · 2018-03-27T01:15:12Z

I can try to make a sample to reproduce as well, but I also am getting the same error.

minuz · 2018-03-27T09:31:03Z

Intertesting. I'm trying to reproduce using a similar pattern that I'm using on my app and it's failing with a different problem. It's now related to the input. If I remove the input from the directive, it loads the component.
I can publish the repo if you want to take a look. I might be doing something silly... =p

Edit: Repo with sample project with issue
https://github.com/minuz/ng-dynamic-component-bug-repro.git

gund · 2018-03-27T17:20:14Z

Hey @minuz, I just quickly had a look at your repro (thanks for taking time creating it) and found that you are using there fixed version of this lib v2.0.3 which is the version before update we are talking about here...

I will try pulling it and upgrading version ta latest but just heads up =)

gund · 2018-03-27T18:46:55Z

Alright, so I pulled your repo, updated lib version to latest (which is v2.1.0) and started app.

First error I've got was some providers not available, so I quickly figured that module was not imported properly: you have to at least once import with DynamicModule#withComponents() method for it to generate all required providers. So by just changing that I got app working and displaying the component:

Note that all console messages were printed correctly without any errors whatsoever.

The only change was in src/app/dynamic-container/dynamic-container.module.ts:

 @NgModule({
-  imports: [CommonModule, DynamicModule],
+  imports: [CommonModule, DynamicModule.withComponents([])],
   declarations: [DynamicContainerComponent],
   exports: [DynamicContainerComponent]
 })

Really have no idea why you are having such errors...

UPDATE: This is where the change was made: https://github.com/minuz/ng-dynamic-component-bug-repro/blob/cec56830964b5b1881bb959e8c5f3b4cf4302f82/src/app/dynamic-container/dynamic-container.module.ts#L10

jzoss · 2018-03-27T19:21:09Z

So.. I have not been able to recreate it yet.. besides on my project that is creating the problem..

So what I see is differ is null on line 315 of the .es5.js file.. causing the line at 317 throw an exception.

Also in my case at least .. It seems that that function is called multiple times.. the first it it goes through it works.. after that differ is null..

gund · 2018-03-27T19:56:12Z

@jzoss that one I got but as you can see it looks like some really edge case because so far every example works fine.

If you have access to code base that is failing try to find some non-trivial special pieces of code that is related to this lib and we can move from there.

minuz · 2018-03-27T21:53:09Z

Thanks for fixing the bug repo...but this change didn't fix the issues I'm having on my app... =/

gund · 2018-03-27T23:11:20Z

@minuz yeah I see.

Are you able to share the project were you are getting error or at least cut particular piece that is failing?

minuz · 2018-03-28T00:12:37Z

@gund, unfortunately no. This is not an open source project and it's infrastructure is confidential IP and under nda. :(

gund · 2018-03-28T11:36:01Z

Anyone else maybe able to create a demo that showcases this error?
Without it I don't think we can move forward...

hassanasad · 2018-03-28T15:27:16Z

@gund - From my test i think if you call the component setup code more than once you get the error mentioned above. First time it works just fine, calling it second time it throws 'forEachItem' of null error.
It used to work fine in previous version though.

So for instance:

setupHeaderComponent() {
    this.headerComponent = {
        component: SomeHeaderComponent, 
        inputs   : { 
            previewUrl  : 'someValue'
        }, 
        outputs  : { 
            eventButtonClicked: (event) => this.headerButtonClicked(event),
        }
    };
}

If i call the above function twice it will throw error. I hope it helps in debugging the issue.

hassanasad · 2018-03-30T15:32:08Z

@gund Did my comment above help debugging this issue ?

gund · 2018-03-30T19:15:50Z

@hassanasad thanks for providing this info.

I did not have time to check it yet, but will try in a few days.

minuz · 2018-04-05T02:45:23Z

I tried to use symlink and make quick changes to check where the problem is.
When doing some null check to prevent then error previously mentioned, I start getting this:

NgxFormElementComponent.html:11 ERROR Error: StaticInjectorError(AppModule)[DynamicDirective -> KeyValueDiffers]: 
  StaticInjectorError(Platform: core)[DynamicDirective -> KeyValueDiffers]: 
    NullInjectorError: No provider for KeyValueDiffers!
    at _NullInjector.get (core.js:1002)
    at resolveToken (core.js:1300)
    at tryResolveToken (core.js:1242)
    at StaticInjector.get (core.js:1110)
    at resolveToken (core.js:1300)
    at tryResolveToken (core.js:1242)
    at StaticInjector.get (core.js:1110)
    at resolveNgModuleDep (core.js:10854)
    at NgModuleRef_.get (core.js:12087)
    at resolveNgModuleDep (core.js:10854)

Any ideias?

lukaszbachman · 2018-04-11T05:00:59Z

I started receiving the same issue only after I upgraded Angular (to 5.2.9) & Angular CLI (to 1.7.4).
Once the upgrade was complete I was still using ng-dynamic-component ^2.0.3 and I could see that error. So I upgraded ng-dynamic-component to 2.1.0 but that didn't help either. Hope that helps to track it down.

gund · 2018-04-11T09:01:00Z

@lukaszbachman so you mean that the problem occurs while you update the version of Angular?
Could you please tell from which version specifically you upgraded?

Maybe there are some minor breaking changes between those versions of Angular...

lukaszbachman · 2018-04-11T10:30:17Z

@gund , that's exactly right. Details about my migration:

Angular from 5.1.0 to 5.2.9
CLI from 1.6.0 to 1.7.4

minuz · 2018-04-11T11:26:01Z

@lukaszbachman, I'm running the same version as you and still works with version 2.0.3 and breaks on version 2.1.0.

this is my package.json

  "dependencies": {
    "@angular/animations": "^5.2.9",
    "@angular/cdk": "^5.2.4",
    "@angular/common": "^5.2.9",
    "@angular/compiler": "^5.2.9",
    "@angular/core": "^5.2.9",
    "@angular/forms": "^5.2.9",
    "@angular/http": "^5.2.9",
    "@angular/material": "^5.2.4",
    "@angular/material-moment-adapter": "^5.2.4",
    "@angular/platform-browser": "^5.2.9",
    "@angular/platform-browser-dynamic": "^5.2.9",
    "@angular/platform-server": "^5.2.9",
    "@angular/router": "^5.2.9",
    "@herbis/ngx-modal": "^0.1.0",
    "animate.css": "^3.6.1",
    "applicationinsights-js": "^1.0.15",
    "bootstrap": "3.3.7",
    "chart.js": "2.7.1",
    "core-js": "^2.5.3",
    "dropzone": "^5.4.0",
    "gl-ngx-configuration": "^0.1.0-alpha.11",
    "html2canvas": "^1.0.0-alpha.10",
    "jquery": "3.1.0",
    "jquery-slimscroll": "1.3.8",
    "jquery-sparkline": "2.4.0",
    "jquery-ui-dist": "1.12.1",
    "jquery.fancytree": "2.27.0",
    "jvectormap": "1.2.2",
    "ladda": "^1.0.6",
    "lodash": "^4.17.5",
    "moment": "^2.20.1",
    "ng-dynamic-component": "2.0.3",
    "ng2-charts": "1.6.0",
    "ng2-select2": "^1.0.0-beta.11",
    "ngx-avatar": "^2.9.0",
    "ngx-bootstrap": "^2.0.3",
    "ngx-dropzone-wrapper": "^5.3.5",
    "ngx-slimscroll": "^3.7.0",
    "ngx-toastr": "^8.3.0",
    "peity": "3.2.1",
    "rxjs": "^5.5.7",
    "select2": "4.0.3",
    "sonarqube-scanner": "^2.0.2",
    "spinkit": "1.2.5",
    "summernote": "0.8.8",
    "tslib": "^1.9.0",
    "uuid": "3.1.0",
    "viz.js": "^1.8.0",
    "workbench-full-client-ng": "^1.2.0-alpha.60",
    "xregexp": "4.0.0",
    "zone.js": "^0.8.20"
  },
  "devDependencies": {
    "@angular/cli": "^1.7.4",
    "@angular/compiler-cli": "^5.2.9",
    "@types/applicationinsights-js": "^1.0.5",
    "@types/jasmine": "^2.8.6",
    "@types/jquery.fancytree": "^2.7.33",
    "@types/jquery.slimscroll": "1.3.31",
    "@types/ladda": "^0.9.30",
    "@types/lodash": "^4.14.104",
    "@types/node": "^9.4.6",
    "@types/select2": "^4.0.41",
    "@types/uuid": "^3.4.3",
    "@types/xregexp": "^3.0.29",
    "codelyzer": "^4.1.0",
    "husky": "^0.14.3",
    "jasmine-core": "^2.99.1",
    "jasmine-spec-reporter": "^4.2.1",
    "karma": "^2.0.0",
    "karma-chrome-launcher": "^2.2.0",
    "karma-cli": "^1.0.1",
    "karma-coverage-istanbul-reporter": "^1.4.1",
    "karma-jasmine": "^1.1.1",
    "karma-jasmine-html-reporter": "^0.2.2",
    "lint-staged": "^7.0.4",
    "node-sass": "^4.8.3",
    "npm-run-all": "^4.1.2",
    "prettier": "1.11.1",
    "protractor": "5.2.0",
    "sass-export": "^1.0.2",
    "shx": "0.2.2",
    "ts-node": "3.3.0",
    "tslint": "5.8.0",
    "tslint-sonarts": "^1.6.0",
    "typescript": "2.6.2"
  }

lukaszbachman · 2018-04-11T12:40:54Z

@gund , @minuz , sorry guys but I think I might have reported a false issue. Inspired by your questions I reverted my Angular+CLI migration, did it again but this time enforced version 2.0.3 on ng-dynamic-component. I can confirm that this works with:

Angular CLI: 1.7.4
Node: 9.11.1
OS: win32 x64
Angular: 5.2.9

I must have upgraded ng-dynamic-component by mistake when upgating my libs, because I had caret range applied to it: ^2.0.3

You can disregard my previous comments on this issue. Thanks for a brainstorm which helped me to recover from that.

skermajo · 2018-04-17T21:45:10Z

I'm seeing the 'differ is null' error similar to this issue if I reinitialize the dynamic component input object. In my specific case, this occurs when a dynamic component input object is backed by an ngrx store, and the store updates.

I've created a stack-blitz that can reproduce the error. Click the 'Reinitialize Input' button more than once and the 'differ is null' error will appear in the console.

https://stackblitz.com/edit/angular-qhxm5e

gund · 2018-04-19T09:31:32Z

@skermajo thank you so much for clear example.

It's now completely clear what is the problem.
Basically it's an edge case when you do update your inputs object (so angular will trigger change detection) while actual object is still the same as before, and so differ will of course be null.

I will create a test case for this particular scenario and fix it so it never happens again =)

gund · 2018-04-19T10:51:53Z

Just a heads up before releasing this fix I have to fix releasing itself in travis CI because after upgrade of semantic-release it got broken... Yeah...

UPDATE: You can track it here: #122

…change detection on them Because object structure might not change while the reference will closes #111

gund · 2018-04-19T13:33:50Z

🎉 This issue has been resolved in version 2.1.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

jzoss · 2018-04-19T20:44:09Z

Awesome.... Thanks!

minuz · 2018-04-30T06:46:23Z

Hi @gund,
Just tested today and can confirm my issue is gone! Thanks for your effort on make this work!
cheers!!

gund added the bug label Apr 19, 2018

gund self-assigned this Apr 19, 2018

gund pushed a commit that referenced this issue Apr 19, 2018

fix(directive): Check if inputs really changed when angular triggers …

14c953c

…change detection on them Because object structure might not change while the reference will closes #111

gund mentioned this issue Apr 19, 2018

Fix inputs reassignment #123

Merged

gund closed this as completed in cc91db3 Apr 19, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade to version 2.1.0 breaks #111

Upgrade to version 2.1.0 breaks #111

minuz commented Mar 26, 2018 •

edited

Loading

gund commented Mar 26, 2018

minuz commented Mar 26, 2018

gund commented Mar 26, 2018 •

edited

Loading

st-clair-clarke commented Mar 26, 2018 •

edited

Loading

gund commented Mar 26, 2018

minuz commented Mar 26, 2018

jzoss commented Mar 27, 2018 •

edited

Loading

minuz commented Mar 27, 2018 •

edited

Loading

gund commented Mar 27, 2018

gund commented Mar 27, 2018 •

edited

Loading

jzoss commented Mar 27, 2018

gund commented Mar 27, 2018

minuz commented Mar 27, 2018

gund commented Mar 27, 2018

minuz commented Mar 28, 2018

gund commented Mar 28, 2018

hassanasad commented Mar 28, 2018 •

edited

Loading

hassanasad commented Mar 30, 2018

gund commented Mar 30, 2018

minuz commented Apr 5, 2018

lukaszbachman commented Apr 11, 2018

gund commented Apr 11, 2018

lukaszbachman commented Apr 11, 2018

minuz commented Apr 11, 2018

lukaszbachman commented Apr 11, 2018

skermajo commented Apr 17, 2018 •

edited

Loading

gund commented Apr 19, 2018

gund commented Apr 19, 2018 •

edited

Loading

gund commented Apr 19, 2018

jzoss commented Apr 19, 2018

minuz commented Apr 30, 2018

Upgrade to version 2.1.0 breaks #111

Upgrade to version 2.1.0 breaks #111

Comments

minuz commented Mar 26, 2018 • edited Loading

gund commented Mar 26, 2018

minuz commented Mar 26, 2018

gund commented Mar 26, 2018 • edited Loading

st-clair-clarke commented Mar 26, 2018 • edited Loading

gund commented Mar 26, 2018

minuz commented Mar 26, 2018

jzoss commented Mar 27, 2018 • edited Loading

minuz commented Mar 27, 2018 • edited Loading

gund commented Mar 27, 2018

gund commented Mar 27, 2018 • edited Loading

jzoss commented Mar 27, 2018

gund commented Mar 27, 2018

minuz commented Mar 27, 2018

gund commented Mar 27, 2018

minuz commented Mar 28, 2018

gund commented Mar 28, 2018

hassanasad commented Mar 28, 2018 • edited Loading

hassanasad commented Mar 30, 2018

gund commented Mar 30, 2018

minuz commented Apr 5, 2018

lukaszbachman commented Apr 11, 2018

gund commented Apr 11, 2018

lukaszbachman commented Apr 11, 2018

minuz commented Apr 11, 2018

lukaszbachman commented Apr 11, 2018

skermajo commented Apr 17, 2018 • edited Loading

gund commented Apr 19, 2018

gund commented Apr 19, 2018 • edited Loading

gund commented Apr 19, 2018

jzoss commented Apr 19, 2018

minuz commented Apr 30, 2018

minuz commented Mar 26, 2018 •

edited

Loading

gund commented Mar 26, 2018 •

edited

Loading

st-clair-clarke commented Mar 26, 2018 •

edited

Loading

jzoss commented Mar 27, 2018 •

edited

Loading

minuz commented Mar 27, 2018 •

edited

Loading

gund commented Mar 27, 2018 •

edited

Loading

hassanasad commented Mar 28, 2018 •

edited

Loading

skermajo commented Apr 17, 2018 •

edited

Loading

gund commented Apr 19, 2018 •

edited

Loading