Skip to content

Dynamic components aren't hydrated when root component is used as an anchor #51157

Closed

Description

@devknoll

Which @angular/* package(s) are the source of the bug?

platform-browser

Is this a regression?

No

Description

As in the linked reproduction, I'm dynamically creating a component like so:

import('./foo/foo.component').then((m) => {
  this.vcr.createComponent(m.FooComponent);
});

The component correctly appears in the SSR output. However, Angular does not appear to hydrate the component. Instead, it creates a new instance, and leaves the existing copy in the DOM dehydrated (and thus, broken), even after hydration completes.

Please provide a link to a minimal reproduction of the bug

https://codesandbox.io/p/sandbox/unruffled-dawn-dngll7

Please provide the exception or error you saw

No response

Please provide the environment you discovered this bug in (run ng version)

No response

Anything else?

No response

Activity

  1. added this to the needsTriage milestone on Jul 24, 2023
  2. JeanMeche commented on Jul 24, 2023

    @JeanMeche
    Member

    ViewContainerRef.createComponent adds the component as sibling of the curremnt component. This means when you use ViewContainerRef.createComponent in the root component, the component is created outside the Angular app (It's created outside the <app-root></app-root>.

    There is no issue when you do the samething in any other compoent !

  3. added
    P3An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
    on Jul 24, 2023
  4. modified the milestones: needsTriage, Backlog on Jul 24, 2023
  5. AndrewKushnir commented on Jul 24, 2023

    @AndrewKushnir
    Contributor

    I've performed an initial investigation and can confirm what @JeanMeche mentioned above: the problem only happens in a root components when inject(ViewContainerRef) is used, thus transforming a value the first slot of a root LView to LContainer (from a component's LView). Related code: https://github.com/angular/angular/blob/main/packages/core/src/hydration/annotate.ts#L101-L122. The current logic basically skips serialization of all views within a root view container. The correct logic should handle such a case and go over all views within a view container recursively (since containers might potentially have nested view containers as well).

    There is also 1 limitation that we would likely need to add: such view containers can not have embedded views in them (i.e. views create via ViewContainerRef.createEmbeddedView), since there is no host element there where we can place ngh id to refer to serialized info. There is a comment node (acting as a host node) available for those views, but the current hydration logic can't use those nodes as a container for ngh id info yet (we'd to support those cases in the future though for partial per-view hydration).

  6. changed the title [-]Dynamic components aren't hydrated[/-] [+]Dynamic components aren't hydrated when root component is used as an anchor[/+] on Jul 24, 2023
  7. AndrewKushnir commented on Jul 24, 2023

    @AndrewKushnir
    Contributor

    Note (to myself): here is a new TestBed test (can be added to the packages/platform-server/test/hydration_spec.ts) that reproduces the issue:

          it('should hydrate dynamically created components using root component as an anchor', async () => {
            @Component({
              standalone: true,
              imports: [CommonModule],
              selector: 'dynamic',
              template: `
                <span>This is a content of a dynamic component.</span>
                <b *ngIf="isServer">This is a SERVER-ONLY content</b>
                <i *ngIf="!isServer">This is a CLIENT-ONLY content</i>
              `,
            })
            class DynamicComponent {
              isServer = isPlatformServer(inject(PLATFORM_ID));
            }
    
            @Component({
              standalone: true,
              selector: 'app',
              template: `
                <main>Hi! This is the main content.</main>
              `,
            })
            class SimpleComponent {
              vcr = inject(ViewContainerRef);
    
              ngAfterViewInit() {
                const compRef = this.vcr.createComponent(DynamicComponent);
                compRef.changeDetectorRef.detectChanges();
              }
            }
    
            const html = await ssr(SimpleComponent);
            let ssrContents = getAppContents(html);
    
            expect(ssrContents).toContain('<app ngh');
    
            resetTViewsFor(SimpleComponent, DynamicComponent);
    
            const appRef = await hydrate(html, SimpleComponent);
            const compRef = getComponentRef<SimpleComponent>(appRef);
            appRef.tick();
    
            ssrContents = stripExcessiveSpaces(stripUtilAttributes(ssrContents, false));
    
            // We expect to see SERVER content, but not CLIENT.
            expect(ssrContents).not.toContain('<i>This is a CLIENT-ONLY content</i>');
            expect(ssrContents).toContain('<b>This is a SERVER-ONLY content</b>');
    
            const clientRootNode = compRef.location.nativeElement;
    
            await whenStable(appRef);
    
            const clientContents =
                stripExcessiveSpaces(stripUtilAttributes(clientRootNode.outerHTML, false));
    
            // After the cleanup, we expect to see CLIENT content, but not SERVER.
            expect(clientContents).toContain('<i>This is a CLIENT-ONLY content</i>');
            expect(clientContents).not.toContain('<b>This is a SERVER-ONLY content</b>');
          });
    
  8. hiepxanh commented on Aug 1, 2023

    @hiepxanh
    Contributor

    Hi, I'm using ngx-dynamic-hooks which is have the same logic. Which have the same issue because of the recreation of angular component which cause by hydration logic. Can I suggest some kind of attribute mark which notice that the Hydration should do nothing with that component?

    p/s: I'm using old day workaround to solve this problem Angular-Dynamic-Hooks/ngx-dynamic-hooks#34 (comment)

  9. 1 remaining item

  10. AndrewKushnir commented on Aug 2, 2023

    @AndrewKushnir
    Contributor

    @hiepxanh I've created a fix for the problem reported in this ticket, see PR #51247. You can test (locally) if this fix also resolves the problem that you've mentioned by using build artifacts from the PR. You can try updating your package.json file and change the reference to the @angular/core package and running npm install --force:

    "dependencies": {
      ...
      "@angular/core": "https://output.circle-artifacts.com/output/job/5457f352-d3e1-4417-943e-6a341dac8930/artifacts/0/angular/core-pr51247-5386d318c4.tgz",
      ...
    }
    

    Note: this archive is available only temporarily, please do not use it in production builds.

  11. hiepxanh commented on Aug 2, 2023

    @hiepxanh
    Contributor

    @AndrewKushnir Oh my god! I'm really lucky to meet you, sir. Every time I get stuck in a hard issue, you just appear like a god. It has been 1 year since the last issue #46473 (comment) and we meet each other again. Thank you so much for this support. It really works and the performance has improved a lot.
    This will be really useful for drag and drop or some Angular builder.
    Since Angular 2 beta and Angular JS, I never lost faith among a lot of JS frameworks. And after 10 years of working with Angular, my knowledge is still usable in this version.

    Thank you so much, it works! I can confirm it worked!
    I hope this fix can land in the next version.

    Before change, there is a flick which lost and generate the HTML again.

    image

    After change, it stable no flick:
    image

  12. AndrewKushnir commented on Aug 2, 2023

    @AndrewKushnir
    Contributor

    @hiepxanh thanks for your kind words!

    You did a great job tracking down the root cause of the problem to this ticket 👍

    Great to hear that the fix resolves the issue in your application and you see performance improvements. The fix would go though the regular process (code review, testing and merging) and will be available in one of the upcoming Angular weekly releases. I'll keep this thread updated once the fix is released.

    Thank you.

  13. added a commit that references this issue on Aug 7, 2023
    9f490da
  14. AndrewKushnir commented on Aug 9, 2023

    @AndrewKushnir
    Contributor

    @hiepxanh just wanted to let you know that Angular v16.2.0 is out (available on NPM) and this fix is included into the release. Thank you.

  15. hiepxanh commented on Aug 10, 2023

    @hiepxanh
    Contributor

    I can confirm it work in the codesandbox.io https://codesandbox.io/p/sandbox/relaxed-feather-27fv8y with angular 16.2.0 version. Thank you for your support @AndrewKushnir

  16. hiepxanh commented on Aug 10, 2023

    @hiepxanh
    Contributor

    @AndrewKushnir thank you so much sir. But I think it lead to other issue. No bug no life, I think so.
    Anyway can you take a look on this issue?
    #51318

  17. angular-automatic-lock-bot commented on Sep 29, 2023

    @angular-automatic-lock-bot

    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.

  18. locked and limited conversation to collaborators on Sep 29, 2023
  19. added a commit that references this issue on Jan 23, 2024
    95ee07e
  20. added a commit that references this issue on Jan 31, 2024
    55965cb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

P3An issue that is relevant to core functions, but does not impede progress. Important, but not urgentarea: coreIssues related to the framework runtimecore: hydrationstate: has PR

Type

No type

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions