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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

ivy: failed to create dynamic component #34152

Closed
mattlewis92 opened this issue Nov 29, 2019 · 4 comments
Closed

ivy: failed to create dynamic component #34152

mattlewis92 opened this issue Nov 29, 2019 · 4 comments

Comments

@mattlewis92
Copy link
Contributor

@mattlewis92 mattlewis92 commented Nov 29, 2019

馃悶 bug report

Affected Package

The issue is caused by package @angular/core

Is this a regression?

Yes, it worked fine in v8

Description

Trying to create a dynamic component fails to work when calling insert on a ViewContainerRef with a component instance's hostView

馃敩 Minimal Reproduction

git clone https://github.com/mattlewis92/ivy-dynamic-component-bug.git
cd ivy-dynamic-component-bug
npm i
npm start -- --open

then click the "click me" button and you'll see the error thrown

馃敟 Exception or Error


ERROR Error: ASSERTION ERROR: index must be positive
    at throwError (core.js:1229)
    at assertGreaterThan (core.js:1213)
    at ViewContainerRef_._adjustIndex (core.js:17341)
    at ViewContainerRef_.insert (core.js:17264)
    at ViewContainerRef_.move (core.js:17288)
    at ViewContainerRef_.insert (core.js:17264)
    at SafeSubscriber._next (modal-keeper.component.ts:24)
    at SafeSubscriber.__tryOrUnsub (Subscriber.js:183)
    at SafeSubscriber.next (Subscriber.js:122)

馃實 Your Environment

Angular Version:


Angular CLI: 9.0.0-rc.4
Node: 12.10.0
OS: darwin x64

Angular: 9.0.0-rc.4
... animations, cli, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router
Ivy Workspace: Yes

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.900.0-rc.4
@angular-devkit/build-angular     0.900.0-rc.4
@angular-devkit/build-optimizer   0.900.0-rc.4
@angular-devkit/build-webpack     0.900.0-rc.4
@angular-devkit/core              9.0.0-rc.4
@angular-devkit/schematics        9.0.0-rc.4
@ngtools/webpack                  9.0.0-rc.4
@schematics/angular               9.0.0-rc.4
@schematics/update                0.900.0-rc.4
rxjs                              6.5.3
typescript                        3.6.4
webpack                           4.41.2

Anything else relevant?
nope

@pkozlowski-opensource

This comment has been minimized.

Copy link
Member

@pkozlowski-opensource pkozlowski-opensource commented Nov 29, 2019

@mattlewis92 thnx for the reproduction - this is a regression that we've introduced in rc.4 and the fix will be trivial. Having said this, you are hitting this particular issue because you are attaching your component host view to two different view containers. As the result there is more processing happening (ex. a view is attached to the first container which results in attaching things to the DOM, modifying internal data structures etc.). Then, a given view is attached to the second container so the DOM needs to be cleared, internal data structures cleared etc. just to do the same work ... again.

So, the suggestion here would be to do:

const component = componentFactory.create(injector)

instead of:

const component = this.container.createComponent(componentFactory);

This will also allow you to drop a query for a ViewContainerRef. Less code and processing for the win :-)
Documentation here: https://angular.io/api/core/ComponentFactory

Again, this is clearly a bug that we will fix asap, but the above is just a suggestion to work-around this particular bug right now and speed up processing as a bonus!

@mattlewis92

This comment has been minimized.

Copy link
Contributor Author

@mattlewis92 mattlewis92 commented Nov 29, 2019

Hey @pkozlowski-opensource, thanks a lot for getting back to me so quickly on this!

We're actually already using const component = componentFactory.create(injector) for all new code, it's just that we have a lot of legacy code still using const component = this.container.createComponent(componentFactory); so migrating and testing all of that will take a lot of time 馃槃

@pkozlowski-opensource

This comment has been minimized.

Copy link
Member

@pkozlowski-opensource pkozlowski-opensource commented Nov 29, 2019

Marking it as a release blocker since it turned out to be a scenario (moving views between containers) that we don't support properly in ivy.

pkozlowski-opensource added a commit to pkozlowski-opensource/angular that referenced this issue Nov 29, 2019
pkozlowski-opensource added a commit to pkozlowski-opensource/angular that referenced this issue Dec 2, 2019
pkozlowski-opensource added a commit to pkozlowski-opensource/angular that referenced this issue Dec 3, 2019
@mhevery mhevery closed this in 5a52990 Dec 4, 2019
@angular-automatic-lock-bot

This comment has been minimized.

Copy link

@angular-automatic-lock-bot angular-automatic-lock-bot bot commented Jan 4, 2020

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jan 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can鈥檛 perform that action at this time.