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

feat(DomRenderer): allow partial DOM hydration from pre-rendered content #13446

Open
jeffbcross opened this Issue Dec 14, 2016 · 24 comments

Comments

Projects
None yet
@jeffbcross
Contributor

jeffbcross commented Dec 14, 2016

I'm recapping a discussion I just had with @alxhub and @tbosch. This is mostly Tobias' design.

I'm submitting a ... (check one with "x")

[x] feature request

Current behavior

Typically when a page is pre-rendered, such as with Universal, the Angular application bootstraps in the browser, then blows away the pre-rendered content, and replaces it with newly created nodes. The DOM tree that is pre-rendered is often very similar, or identical to the DOM tree created on the front end, but there are issues that make it difficult for Angular to take over the existing DOM tree (otherwise referred to as DOM hydration).

The actual handoff of destroying the old DOM and showing the new DOM is pretty fast and seamless, so it's not necessarily a major UX issue in and of itself. Where it becomes problematic is in cases like ads that load in iframes (which is pretty much all display ads). If these ad iframes are pre-rendered -- which is a business requirement for many publishers -- and the iframe gets moved in the DOM, the iframe will refresh. This causes some ad networks to suspect abuse, as if publishers are trying to sneak more ad views.

Why Not Use the Already-Rendered DOM?

One issue is that with asynchronicity+conditional DOM (i.e. *ngIf="data | async"), the tree in the client may be rendered before the condition is truthy, whereas the pre-rendered version may have the full tree with async data resolved.

Another challenge is that text nodes are not able to be selected by CSS selectors, which would mean the renderer would have to rely on child ordering in order to associate pre-rendered nodes with client-rendered nodes (which is not always correct). Similar challenge goes for elements in an *ngFor, the order must be assumed to be identical.

The renderer would also be responsible for cleaning up pre-rendered orphan nodes. i.e. if 30 items in an *ngFor were pre-rendered, but only 20 were rendered in the client, the additional 10 nodes would need to be removed to prevent unexpected behavior.

Proposal: Optional, explicit, partial DOM Hydration

Allow setting a user-specified attribute on elements to associate the pre-rendered version with client-rendered version. If the renderer comes to a node that it can't associate with an existing node, it will blow away the node and re-create it. The developer would be responsible for setting these ids on the elements they care about. Example:

import { HydrateDomConfig, NgModule } from '@angular/core';

@NgModule({
  providers: [
    { 
      provide: HydrateDomConfig, 
      useValue: {
        hydrate: true, // default false for backwards compat
        attribute: 'pid', // default 'id'
      } 
    }
  ]
})

Component:

@Component({
  template: `
    <div pid="header">
      <header-ad pid="header-ad"></header-ad>
      <div>
        <!-- this will get blown away and re-created since it lacks attribute -->
      </div>
    </div>
  `
})

This design allows the DomRenderer to traverse the DOM tree and match elements for each branch starting at the root until it can't go any deeper, at which point it would blow away the descendants and re-create them.

Text nodes would all be destroyed and re-created with this design, as well as any node that doesn't have the set attribute, pid.

I don't expect that the rendering would be user-perceivable, other than if there are discrepancies between pre-rendered and client-rendered DOM, but that's a concern even without this feature.

CC @gdi2290 @pxwise

@tbosch & @alxhub please feel free to add anything I missed (or misrepresented).

@pxwise

This comment has been minimized.

pxwise commented Dec 14, 2016

Ad wipeout on client bootstrap is a real world issue for us using universal and this hydration proposal should solve it. Our current workaround gets us partway there, moving server DOM into the same position in client rendered DOM but does not outsmart ad verification services that watch for DOM mutations, hiding the ad upon client bootstrap.

Big thumbs up for opt-in hydration.

@gdi2290

This comment has been minimized.

Member

gdi2290 commented Dec 14, 2016

LGTM, Keep in mind this is a huge problem with all js frameworks so for Angular to have a solution is a great win for when comparing different SSR solutions. The real solution is having Ads actually work together with js frameworks but that will never happen, other than AMP. I can see how it would be implemented by rewriting selecting root element and providing a different render path.

For the |async you definitely have that problem with Promises due to the microtask while Observables do return synchronously. On the client, we can assume there will be batched data sent from the server to the client which gives us all of the results immediately. For Universal, we need to have the data available to the client synchronously anyways to reuse the server Http responses

var res = {data: 'ok'};
var api = Rx.Observable.of(res);
var api2 = Promise.resolve(res);

var vm = {};
var vm2 = {};
api.subscribe(function(data) { vm = data; });
api2.then(function(data) { vm2 = data; });
console.log(vm); // {data: 'ok'}
console.log(vm2); // {}

github-tipe-logo

@jeffbcross

This comment has been minimized.

Contributor

jeffbcross commented Dec 14, 2016

An alternative design would be to provide an alternate renderer that would extend DomRenderer, rather than modifying DomRenderer to behave differently depending on the hydrate value.

@NgModule({
  providers: [{
    provide: Renderer, useClass: DomHydrationRenderer
  }]
})
@gdi2290

This comment has been minimized.

Member

gdi2290 commented Dec 15, 2016

+1 for DomHydrationRenderer

github-tipe-logo

@IgorMinar

This comment has been minimized.

Member

IgorMinar commented Dec 16, 2016

Two things:

  • angular.io v42 might also benefit from this. I had to put in a bunch of code that prevented some flickering caused by the content being wiped out by the renderer code when initializing components
  • I wonder if there is something from the AMP solution that we could leverage to in universal, it would be good to look into that before we go too creative with our own solution/workaround.
@playground

This comment has been minimized.

playground commented Dec 19, 2016

+1 either solution will be beneficial and vital for us to move forward.

@tbosch

This comment has been minimized.

Member

tbosch commented Dec 28, 2016

We can still make the work generically, when using auto generated ids based on the place in the element hierarchy (and not based on the creation order).

I think we should think this through as well before we decide.

@FahadMullaji

This comment has been minimized.

FahadMullaji commented Jan 10, 2017

is HydrateDomConfig part of @angular/core? I get error when I try to compile the application.

error TS2305: Module '"/home/fahad/Workspace/siteCuriouss/node_modules/@angular/core/index"' has no exported member 'HydrateDomConfig'.

@DzmitryShylovich

This comment has been minimized.

Contributor

DzmitryShylovich commented Jan 10, 2017

@FahadMullaji it's only a proposal.

@josephliccini

This comment has been minimized.

josephliccini commented Aug 29, 2017

This proposal is super exciting!

Has any thought been given to pre-rendered lazy routes? We can achieve pre rendered lazy routes via https://github.com/angular/universal/blob/master/modules/ng-module-map-ngfactory-loader/README.md

Is it possible for the hydration to occur after lazy route is fetched and then rendering begins?

Just curious on thoughts here.

Universal has been great and straightforward to use so far; thanks to everyone involved!

@mcferren

This comment has been minimized.

mcferren commented Sep 24, 2017

What couples the ComponentRef obj to the Dom string (cmpref.changeDetectorRef.rootNodes[0] & cmpref.hostView.rootNodes[0] ect)? Is there an explicit reference(i.e. dom string selector) or does this binding live in memory? Is there a possibility of calling createComponent with an existing dom node as an argument?

@vytautas-pranskunas-

This comment has been minimized.

vytautas-pranskunas- commented Aug 18, 2018

Any updates on this - i have big issue with this as everybody here :(

@FahadMullaji

This comment has been minimized.

FahadMullaji commented Aug 20, 2018

@vytautas-pranskunas-

This comment has been minimized.

vytautas-pranskunas- commented Aug 20, 2018

@gdi2290

This comment has been minimized.

Member

gdi2290 commented Aug 20, 2018

@vytautas-pranskunas-
Every framework, other than angular, has solved this problem. With that said it might not be a problem once the new ivy renderer is released.

@vytautas-pranskunas-

This comment has been minimized.

vytautas-pranskunas- commented Aug 21, 2018

@naveedahmed1

This comment has been minimized.

naveedahmed1 commented Aug 22, 2018

For the ads, I think the ad code shouldn't initialize on sever, if it does I think it would be a policy issue for most of the ad networks/advertisers including Google Adsense.

For this we can check and include ad code only when in browser.

You can check if its running in browser

import { PLATFORM_ID } from '@angular/core';
import { isPlatformBrowser } from '@angular/common';

private isBrowser = isPlatformBrowser(this.platformId);

constructor(@Inject(PLATFORM_ID) private platformId) { }

Adding ad code to SPA is little tricky, if your page is making an ajax request and main content of the page depends on completion of that request you should also wait for that request to complete before you add ad code to the page. Otherwise, if ad code is initialized before ajax request completes (and contents are populated) it would also cause ad network policy issue.

@gdi2290

This comment has been minimized.

Member

gdi2290 commented Aug 22, 2018

yeah the issue isn't rendering ads on the server so much as trying to make sure angular doesn't remove/edit/change the ssr element. There was a solution which reattach the ad dom from the server to the new dom created by the client. Again the problem with that is the ad detecting dom manipulation. So the only reason why people want hydration is for ads since it's always faster just to replace the ssr view with csr view. There were solutions made to also keep the element in the dom and insert/remove around that element but would require some rewriting of the renderer to keep track of the elements.

Ivy render will likely solve this since you can choose which elements you want to boot angular into and ignore the ad elements.

@vytautas-pranskunas-

This comment has been minimized.

vytautas-pranskunas- commented Aug 22, 2018

Adds are not only reason why people want hydration. Hydration needed for prerendering because in scenario when page content is fetched after bootstrapping SSR has no flashing but prerendered. So hydrations needed for prerendered pages also and not sure if ignoring will solve this problem or you will have to ignore whole body because content is everywhere.

I think best solution would be something simmilar like React Virtual DOM which checks what parts where changed and update only those one.

@trotyl

This comment has been minimized.

Contributor

trotyl commented Aug 22, 2018

Hydration is only a lib stuff, nothing tied to the view layer, one can already provide custom Renderer to achieve it.

Clear existing contents is an implementation-specific behavior of default Renderer2#selectRootElement provided in platform-browser.

@vytautas-pranskunas-

This comment has been minimized.

vytautas-pranskunas- commented Aug 22, 2018

Do you have any example of custom Renderer2 implementation and how to use it after?

@trotyl

This comment has been minimized.

Contributor

trotyl commented Aug 22, 2018

@vytautas-pranskunas- There's an article for old V1 Renderer: https://blog.nrwl.io/experiments-with-angular-renderers-c5f647d4fd9e, should be easy to migrate:

  • Renderer -> Renderer2;
  • RootRenderer -> RendererFactory2;
@vytautas-pranskunas-

This comment has been minimized.

vytautas-pranskunas- commented Aug 22, 2018

trotyl thanks for this. However instead forcing all users to write own renderers it would be benifitial for Angular renderer to update only those dom parts that has changes. Because comparing SSR or prerendered DOM with new one is not on day task if we want to do it correctly not just keep SSR content...

  • Do you know how to access SSR rendered content from custom renderer for comparison?
@trotyl

This comment has been minimized.

Contributor

trotyl commented Aug 22, 2018

Yes, the feature request is totally valid, but since it might still need to wait, one can made it in 3rd party library and share to public. Community inputs could also be very beneficial.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment