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

lazy-in-for #26

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ export class LazyElementDynamicDirective implements OnInit {
}

const host = (this.template as any)._def.element.template.nodes[0].element;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't it help to clone the element so that when we change the tag, we're changing it only for this one particular element and not every similar element ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that would be helpful, although im not 100% sure thats the issue and i wasnt completely sure how to create a new TemplateRef from scratch. Its abstract so i cant instantiate a new one and when its injected im not sure how angular is handling that. The TemplateRef seems to be the key though.

So from what i am following in the code, the reason why you've decided to swap out the tag name is because then that element will still have the inputs/outputs that are needed for the web component to bind appropriately to the container component. Instead of changing the tag name, could we just add another html element to the dom and loop through the current elements attributes to get all the bindings?

FYI, this PR didnt work completely. I added more test elements (it worked with 2) to the array but still have the race condition. Will keep looking into a solution.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@janstadt would try to prevent introducing such logic as manual bindings behind the scene cuz that would be prolly very fragile... So what about the case of looping over X elements which are the same with same bindings, works ? Also for cloning I only thought about the last .element object not the whole this.template ? Could help ?

Copy link
Author

@janstadt janstadt Oct 8, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tomastrajan im not sure thats possible. Im running out of ideas. I might be able to just use the other non dynamic loader with a bunch of if checks based on the user response for which modules they have access to. Clearly not optimal but would get me past this issue. It seems like there should be an easier way to do this but it doesnt seem that straight forward. I did try to create a new element like this:

const axLazy = this.template.elementRef.nativeElement.nextSibling;
var newElement = document.createElement(this.tag);
        
        for(let i = 0; i < axLazy.attributes.length; i++) {
            const item = axLazy.attributes.item(i);
            newElement.setAttribute(item.nodeName, item.nodeValue);
        }

        this.renderer.removeChild(this.template.elementRef.nativeElement.parentElement, axLazy);
        this.renderer.appendChild(this.template.elementRef.nativeElement.parentElement, newElement);

At one point nextSibling was valued as the actual ax-lazy-element but now its always null due to the fact that the template hasnt been completely rendered at the time of the check which is unfortunate. Not sure how i was able to get it to be valued but it must have been some weird race condition yet again. Let me know if you are able to get anything working.

Thanks!

host.name = this.tag;

const elementConfig =
this.elementsLoaderService.getElementConfig(this.tag) ||
Expand All @@ -63,6 +62,7 @@ export class LazyElementDynamicDirective implements OnInit {
.loadElement(this.url, this.tag, this.isModule)
.then(() => {
this.vcr.clear();
host.name = this.tag;
this.vcr.createEmbeddedView(this.template);
})
.catch(() => {
Expand Down