refactor(core): avoid ngSkipHydration on nodes other than component host ones (#49500)

AndrewKushnir · pkozlowski-opensource · commit a94b66f432e8 · 2023-03-21T18:59:25.000+01:00
Angular Hydration uses Components as a hydration boundary, i.e. you can enable/disable hydration on per-component basis. This commit enforces that the `ngSkipHydration` can only be applied on component host nodes (an error if thrown otherwise). PR Close #49500
diff --git a/packages/core/src/hydration/error_handling.ts b/packages/core/src/hydration/error_handling.ts
@@ -62,3 +62,11 @@ export function unsupportedProjectionOfDomNodes(): Error {
       'Hydration is not supported for such cases, consider refactoring the code to avoid ' +
       'this pattern or using `ngSkipHydration` on the host element of the component.');
 }
+
+export function invalidSkipHydrationHost() {
+  // TODO: improve error message and use RuntimeError instead.
+  return new Error(
+      'The `ngSkipHydration` flag is applied on a node ' +
+      'that doesn\'t act as a component host. Hydration can be ' +
+      'skipped only on per-component basis.');
+}
diff --git a/packages/core/src/render3/instructions/element.ts b/packages/core/src/render3/instructions/element.ts
@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {validateMatchingNode, validateNodeExists} from '../../hydration/error_handling';
+import {invalidSkipHydrationHost, validateMatchingNode, validateNodeExists} from '../../hydration/error_handling';
 import {locateNextRNode} from '../../hydration/node_lookup_utils';
 import {hasNgSkipHydrationAttr} from '../../hydration/skip_hydration';
 import {getSerializedContainerViews, markRNodeAsClaimedByHydration, setSegmentHead} from '../../hydration/utils';
@@ -17,7 +17,7 @@ import {registerPostOrderHooks} from '../hooks';
 import {hasClassInput, hasStyleInput, TAttributes, TElementNode, TNode, TNodeFlags, TNodeType} from '../interfaces/node';
 import {Renderer} from '../interfaces/renderer';
 import {RElement} from '../interfaces/renderer_dom';
-import {isContentQueryHost, isDirectiveHost} from '../interfaces/type_checks';
+import {isComponentHost, isContentQueryHost, isDirectiveHost} from '../interfaces/type_checks';
 import {HEADER_OFFSET, HYDRATION, LView, RENDERER, TView} from '../interfaces/view';
 import {assertTNodeType} from '../node_assert';
 import {appendChild, clearElementContents, createElementNode, setupStaticAttributes} from '../node_manipulation';
@@ -231,11 +231,17 @@ function locateOrCreateElementNodeImpl(
   // Checks if the skip hydration attribute is present during hydration so we know to
   // skip attempting to hydrate this block.
   if (hydrationInfo && hasNgSkipHydrationAttr(tNode)) {
-    enterSkipHydrationBlock(tNode);
-
-    // Since this isn't hydratable, we need to empty the node
-    // so there's no duplicate content after render
-    clearElementContents(renderer, native);
+    if (isComponentHost(tNode)) {
+      enterSkipHydrationBlock(tNode);
+
+      // Since this isn't hydratable, we need to empty the node
+      // so there's no duplicate content after render
+      clearElementContents(renderer, native);
+    } else if (ngDevMode) {
+      // If this is not a component host, throw an error.
+      // Hydration can be skipped on per-component basis only.
+      throw invalidSkipHydrationHost();
+    }
   }
   return native;
 }
diff --git a/packages/platform-server/test/hydration_spec.ts b/packages/platform-server/test/hydration_spec.ts
@@ -7,7 +7,7 @@
  */
 
 import {CommonModule, DOCUMENT, isPlatformServer, NgComponentOutlet, NgFor, NgIf, NgTemplateOutlet} from '@angular/common';
-import {APP_ID, ApplicationRef, Component, ComponentRef, createComponent, destroyPlatform, ElementRef, EnvironmentInjector, getPlatform, inject, Input, PLATFORM_ID, Provider, TemplateRef, Type, ViewChild, ViewContainerRef, ɵgetComponentDef as getComponentDef, ɵprovideHydrationSupport as provideHydrationSupport, ɵsetDocument, ɵunescapeTransferStateContent as unescapeTransferStateContent} from '@angular/core';
+import {APP_ID, ApplicationRef, Component, ComponentRef, createComponent, destroyPlatform, Directive, ElementRef, EnvironmentInjector, getPlatform, inject, Input, PLATFORM_ID, Provider, TemplateRef, Type, ViewChild, ViewContainerRef, ɵgetComponentDef as getComponentDef, ɵprovideHydrationSupport as provideHydrationSupport, ɵsetDocument, ɵunescapeTransferStateContent as unescapeTransferStateContent} from '@angular/core';
 import {TestBed} from '@angular/core/testing';
 import {bootstrapApplication} from '@angular/platform-browser';
 import {first} from 'rxjs/operators';
@@ -1788,6 +1788,62 @@ describe('platform-server integration', () => {
            verifyAllNodesClaimedForHydration(clientRootNode);
            verifyClientAndSSRContentsMatch(ssrContents, clientRootNode);
          });
+
+      it('should throw when ngSkipHydration attribute is set on a node ' +
+             'which is not a component host',
+         async () => {
+           @Component({
+             standalone: true,
+             selector: 'app',
+             template: `
+                <header ngSkipHydration>Header</header>
+                <footer ngSkipHydration>Footer</footer>
+              `,
+           })
+           class SimpleComponent {
+           }
+
+           try {
+             await ssr(SimpleComponent);
+           } catch (e: unknown) {
+             expect((e as Error).toString())
+                 .toContain(
+                     'The `ngSkipHydration` flag is applied ' +
+                     'on a node that doesn\'t act as a component host');
+           }
+         });
+
+      it('should throw when ngSkipHydration attribute is set on a node ' +
+             'which is not a component host (when using host bindings)',
+         async () => {
+           @Directive({
+             standalone: true,
+             selector: '[dir]',
+             host: {ngSkipHydration: 'true'},
+           })
+           class Dir {
+           }
+
+           @Component({
+             standalone: true,
+             selector: 'app',
+             imports: [Dir],
+             template: `
+                <div dir></div>
+              `,
+           })
+           class SimpleComponent {
+           }
+
+           try {
+             await ssr(SimpleComponent);
+           } catch (e: unknown) {
+             expect((e as Error).toString())
+                 .toContain(
+                     'The `ngSkipHydration` flag is applied ' +
+                     'on a node that doesn\'t act as a component host');
+           }
+         });
     });
 
     describe('corrupted text nodes restoration', () => {
