diff --git a/goldens/public-api/core/errors.api.md b/goldens/public-api/core/errors.api.md index 56e677cc704..4e364583fec 100644 --- a/goldens/public-api/core/errors.api.md +++ b/goldens/public-api/core/errors.api.md @@ -62,6 +62,8 @@ export const enum RuntimeErrorCode { // (undocumented) HYDRATION_MISSING_NODE = -502, // (undocumented) + HYDRATION_MISSING_NODE_ON_PATH = 509, + // (undocumented) HYDRATION_MISSING_SIBLINGS = -501, // (undocumented) HYDRATION_NODE_MISMATCH = -500, @@ -146,6 +148,8 @@ export const enum RuntimeErrorCode { // (undocumented) OUTPUT_REF_DESTROYED = 953, // (undocumented) + PARENT_NODE_NOT_FOUND = 510, + // (undocumented) PIPE_NOT_FOUND = -302, // (undocumented) PLATFORM_ALREADY_DESTROYED = 404, diff --git a/packages/core/src/core_private_export.ts b/packages/core/src/core_private_export.ts index afeff1b5327..5e787eaabad 100644 --- a/packages/core/src/core_private_export.ts +++ b/packages/core/src/core_private_export.ts @@ -96,6 +96,7 @@ export { withIncrementalHydration as ɵwithIncrementalHydration, } from './hydration/api'; export {CACHE_ACTIVE as ɵCACHE_ACTIVE} from './hydration/cache'; +export {describeDomNode as ɵdescribeDomNode} from './hydration/error_handling'; export {withEventReplay as ɵwithEventReplay} from './hydration/event_replay'; export {resetIncrementalHydrationRuntimeForTests as ɵresetIncrementalHydrationRuntimeForTests} from './hydration/incremental_runtime'; export { diff --git a/packages/core/src/errors.ts b/packages/core/src/errors.ts index 84c369cb695..250f436c8ab 100644 --- a/packages/core/src/errors.ts +++ b/packages/core/src/errors.ts @@ -89,6 +89,8 @@ export const enum RuntimeErrorCode { HYDRATION_STABLE_TIMEDOUT = -506, MISSING_SSR_CONTENT_INTEGRITY_MARKER = -507, MISCONFIGURED_INCREMENTAL_HYDRATION = 508, + HYDRATION_MISSING_NODE_ON_PATH = 509, + PARENT_NODE_NOT_FOUND = 510, // Signal Errors SIGNAL_WRITE_FROM_ILLEGAL_CONTEXT = 600, diff --git a/packages/core/src/hydration/error_handling.ts b/packages/core/src/hydration/error_handling.ts index ccfe1fdffbb..89996cf12bc 100644 --- a/packages/core/src/hydration/error_handling.ts +++ b/packages/core/src/hydration/error_handling.ts @@ -244,7 +244,7 @@ export function invalidSkipHydrationHost(rNode: RNode): Error { function stringifyTNodeAttrs(tNode: TNode): string { const results = []; if (tNode.attrs) { - for (let i = 0; i < tNode.attrs.length; ) { + for (let i = 0; i < tNode.attrs.length;) { const attrName = tNode.attrs[i++]; // Once we reach the first flag, we know that the list of // attributes is over. @@ -490,3 +490,14 @@ function shorten(input: string | null, maxLength = 50): string { input = stripNewlines(input); return input.length > maxLength ? `${input.substring(0, maxLength - 1)}…` : input; } + +/** + * Cheap, production-safe description of a DOM node for error messages: its `nodeName`, plus a + * truncated snippet of its `textContent` if it has any. Unlike `shorten()` above, this is meant + * to be called from code paths that also run in production, so it skips the newline-stripping + * and ellipsis formatting that `shorten()` does. + */ +export function describeDomNode(node: Node): string { + const textContent = node.textContent?.slice(0, 50); + return textContent ? `${node.nodeName} ("${textContent}")` : node.nodeName; +} diff --git a/packages/core/src/hydration/node_lookup_utils.ts b/packages/core/src/hydration/node_lookup_utils.ts index afd81aeb7bc..ddff29c5337 100644 --- a/packages/core/src/hydration/node_lookup_utils.ts +++ b/packages/core/src/hydration/node_lookup_utils.ts @@ -6,6 +6,7 @@ * found in the LICENSE file at https://angular.dev/license */ +import {RuntimeError, RuntimeErrorCode} from '../errors'; import {TNode, TNodeType} from '../render3/interfaces/node'; import {RElement, RNode} from '../render3/interfaces/renderer_dom'; import { @@ -184,6 +185,16 @@ export function siblingAfter(skip: number, from: RNode): T | nu let currentNode = from; for (let i = 0; i < skip; i++) { ngDevMode && validateSiblingNodeExists(currentNode); + // `validateSiblingNodeExists` above would normally catch a missing sibling too, but it's + // dev-mode only. Guard against it here so production throws a coded RuntimeError instead + // of a raw TypeError when dereferencing `currentNode` below. + if (currentNode == null) { + throw new RuntimeError( + RuntimeErrorCode.HYDRATION_MISSING_SIBLINGS, + ngDevMode && + 'During hydration Angular expected more sibling nodes to be present. This usually means the client-rendered DOM no longer matches the server-rendered HTML.', + ); + } currentNode = currentNode.nextSibling!; } return currentNode as T; @@ -219,6 +230,16 @@ function navigateToNode(from: Node, instructions: (number | NodeNavigationStep)[ if (ngDevMode && !node) { throw nodeNotFoundAtPathError(from, stringifyNavigationInstructions(instructions)); } + // `nodeNotFoundAtPathError` above would normally catch a missing node too, but it's + // dev-mode only. Guard against it here so production throws a coded RuntimeError instead + // of a raw TypeError when dereferencing `node` below. + if (!node) { + throw new RuntimeError( + RuntimeErrorCode.HYDRATION_MISSING_NODE_ON_PATH, + ngDevMode && + 'During hydration Angular was unable to locate a node using a recorded navigation path. This usually means the client-rendered DOM no longer matches the server-rendered HTML.', + ); + } switch (step) { case NODE_NAVIGATION_STEP_FIRST_CHILD: node = node.firstChild!; @@ -232,6 +253,17 @@ function navigateToNode(from: Node, instructions: (number | NodeNavigationStep)[ if (ngDevMode && !node) { throw nodeNotFoundAtPathError(from, stringifyNavigationInstructions(instructions)); } + // Same as above: `node` can legitimately end up null here (the path's last step ran off + // the end of the real DOM) without the loop ever dereferencing it further, so the raw + // TypeError guard above never gets a chance to fire for this case. Catch it here instead, + // since this function's return type promises a non-null `RNode`. + if (!node) { + throw new RuntimeError( + RuntimeErrorCode.HYDRATION_MISSING_NODE_ON_PATH, + ngDevMode && + 'During hydration Angular was unable to locate a node using a recorded navigation path. This usually means the client-rendered DOM no longer matches the server-rendered HTML.', + ); + } return node as RNode; } diff --git a/packages/core/src/render3/instructions/element.ts b/packages/core/src/render3/instructions/element.ts index 7d5caf585d3..78ac6fb430f 100644 --- a/packages/core/src/render3/instructions/element.ts +++ b/packages/core/src/render3/instructions/element.ts @@ -8,6 +8,7 @@ import {RuntimeError, RuntimeErrorCode} from '../../errors'; import { + describeDomNode, invalidSkipHydrationHost, validateMatchingNode, validateNodeExists, @@ -376,19 +377,12 @@ function locateOrCreateElementNodeImpl( // `hasSkipHydrationAttrOnRElement` below calls `.hasAttribute`, which needs `native` to be // an Element. `validateMatchingNode` above would normally catch a wrong node type, but it's // dev-mode only. Guard against it here too, cheaply, so production throws a coded - // RuntimeError instead of a raw TypeError. In production, skip the full sentence and just - // include the mismatched node's `nodeName`/`textContent` so the error stays cheap to build. + // RuntimeError instead of a raw TypeError. if ((native as unknown as Node).nodeType !== Node.ELEMENT_NODE) { - const node = native as unknown as Node; - // Truncate so a large text node can't blow up the error message, same as - // `shorten()` in `hydration/error_handling.ts`. - const textContent = node.textContent && node.textContent.slice(0, 50); - const description = textContent ? `${node.nodeName} ("${textContent}")` : node.nodeName; throw new RuntimeError( RuntimeErrorCode.HYDRATION_NODE_MISMATCH, - ngDevMode - ? `During hydration Angular expected an element at this location, but found a ${description} node instead.` - : description, + ngDevMode && + `During hydration Angular expected an element at this location, but found a ${describeDomNode(native as unknown as Node)} node instead.`, ); } diff --git a/packages/core/src/render3/node_manipulation.ts b/packages/core/src/render3/node_manipulation.ts index e89ea4df97f..658c57ca0dc 100644 --- a/packages/core/src/render3/node_manipulation.ts +++ b/packages/core/src/render3/node_manipulation.ts @@ -8,6 +8,7 @@ import {consumerDestroy, setActiveConsumer} from '../../primitives/signals'; +import {RuntimeError, RuntimeErrorCode} from '../errors'; import {NotificationSource} from '../change_detection/scheduling/zoneless_scheduling'; import {hasInSkipHydrationBlockFlag} from '../hydration/skip_hydration'; import {ViewEncapsulation} from '../metadata/view'; @@ -486,7 +487,30 @@ function executeOnDestroys(tView: TView, lView: LView): void { * @param tNode: `TNode` for which we wish to retrieve render parent. * @param lView: Current `LView`. */ -export function getParentRElement(tView: TView, tNode: TNode, lView: LView): RElement | null { +export function getParentRElement( + tView: TView, + tNode: TNode | null, + lView: LView, +): RElement | null { + // `tNode` can genuinely be null here, not just as a defensive type-widening measure. An + // `@if`/`@switch` branch's content is its own embedded view with its own `TView`, built the + // first time that branch is rendered. If an error interrupts that first creation pass (for + // example, a hydration mismatch on one of the branch's later nodes, after an earlier node's + // `TNode` was already created), `TView.firstCreatePass` still gets flipped to `false` before + // the error propagates (see `renderView()`'s `catch` block in `instructions/render.ts`), + // permanently marking the view's `TNode` data as "already created" even though it isn't. + // Unlike a component's `TView`, nothing rebuilds an embedded view's `TView` afterward, so the + // next time that same branch is selected, its instructions read directly from the still-null + // slot in `tView.data` instead of creating a fresh `TNode` — which is what surfaces here. + // Guard against it so production throws a coded RuntimeError instead of a raw TypeError when + // dereferencing `tNode.parent` below. + if (tNode === null) { + throw new RuntimeError( + RuntimeErrorCode.PARENT_NODE_NOT_FOUND, + ngDevMode && + 'getParentRElement() was called with a null TNode, so no parent element could be resolved. This usually means a TNode was never created for this node, or was already destroyed.', + ); + } return getClosestRElement(tView, tNode.parent, lView); } diff --git a/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json b/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json index 54cd560acc3..e676e2c62d4 100644 --- a/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json +++ b/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json @@ -431,7 +431,6 @@ "defaultThrowError", "delayChangeDetectionForEvents", "deleteOrUnsetInMap", - "describeDomNode", "destroyLView", "destroyViewTree", "detachMovedView", diff --git a/packages/core/test/bundling/create_component/bundle.golden_symbols.json b/packages/core/test/bundling/create_component/bundle.golden_symbols.json index 4f747dc0989..6fa3a7cdde3 100644 --- a/packages/core/test/bundling/create_component/bundle.golden_symbols.json +++ b/packages/core/test/bundling/create_component/bundle.golden_symbols.json @@ -343,7 +343,6 @@ "defaultErrorHandler", "defaultThrowError", "delayChangeDetectionForEvents", - "describeDomNode", "destroyLView", "destroyViewTree", "detachMovedView", diff --git a/packages/core/test/bundling/defer/bundle.golden_symbols.json b/packages/core/test/bundling/defer/bundle.golden_symbols.json index f119ff5f0b4..10972b74c69 100644 --- a/packages/core/test/bundling/defer/bundle.golden_symbols.json +++ b/packages/core/test/bundling/defer/bundle.golden_symbols.json @@ -43,7 +43,6 @@ "createLinkElement", "createProvidersConfig", "createStyleElement", - "describeDomNode", "errorHandler", "getBaseElementHref", "getDOM", diff --git a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json index ea1786afdd6..f4128fd8914 100644 --- a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json @@ -507,7 +507,6 @@ "defaultIterableDiffersFactory", "defaultThrowError", "delayChangeDetectionForEvents", - "describeDomNode", "destroyLView", "destroyViewTree", "detachMovedView", diff --git a/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json b/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json index b3e44d84dae..a2ea5f25415 100644 --- a/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json @@ -504,7 +504,6 @@ "defaultIterableDiffersFactory", "defaultThrowError", "delayChangeDetectionForEvents", - "describeDomNode", "destroyLView", "destroyViewTree", "detachMovedView", diff --git a/packages/core/test/bundling/hydration/bundle.golden_symbols.json b/packages/core/test/bundling/hydration/bundle.golden_symbols.json index e011ee34966..a9b610cb091 100644 --- a/packages/core/test/bundling/hydration/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hydration/bundle.golden_symbols.json @@ -475,7 +475,6 @@ "defaultThrowError", "deferBlockHasErrored", "delayChangeDetectionForEvents", - "describeDomNode", "destroyLView", "destroyViewTree", "detachMovedView", diff --git a/packages/core/test/bundling/router/bundle.golden_symbols.json b/packages/core/test/bundling/router/bundle.golden_symbols.json index e37b940eaf7..b852f434f57 100644 --- a/packages/core/test/bundling/router/bundle.golden_symbols.json +++ b/packages/core/test/bundling/router/bundle.golden_symbols.json @@ -568,7 +568,6 @@ "defaultUrlMatcher", "defer", "delayChangeDetectionForEvents", - "describeDomNode", "destroyLView", "destroyViewTree", "detachMovedView", diff --git a/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json b/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json index 35f8283fd04..5455e4f8ae0 100644 --- a/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json +++ b/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json @@ -315,7 +315,6 @@ "defaultErrorHandler", "defaultThrowError", "delayChangeDetectionForEvents", - "describeDomNode", "destroyLView", "destroyViewTree", "detachMovedView", diff --git a/packages/platform-browser/src/dom/dom_renderer.ts b/packages/platform-browser/src/dom/dom_renderer.ts index fa212895d3b..ac329f8eae0 100644 --- a/packages/platform-browser/src/dom/dom_renderer.ts +++ b/packages/platform-browser/src/dom/dom_renderer.ts @@ -20,6 +20,7 @@ import { RendererStyleFlags2, RendererType2, ViewEncapsulation, + ɵdescribeDomNode as describeDomNode, ɵRuntimeError as RuntimeError, type ListenerOptions, ɵTracingService as TracingService, @@ -359,11 +360,10 @@ class DefaultDomRenderer2 implements Renderer2 { if (refChild != null && refChild.parentNode !== targetParent) { throw new RuntimeError( RuntimeErrorCode.INSERT_BEFORE_NODE_NOT_FOUND, - ngDevMode - ? `Angular could not insert a node before ${describeDomNode(refChild)} because it is no longer a child of ${describeDomNode(targetParent)}. ` + - `This can happen when code outside of Angular's control (for example, a browser extension or a script that directly manipulates the DOM) ` + - `has moved or removed a node that Angular is still managing.` - : describeDomNode(refChild), + ngDevMode && + `Angular could not insert a node before ${describeDomNode(refChild)} because it is no longer a child of ${describeDomNode(targetParent)}. ` + + `This can happen when code outside of Angular's control (for example, a browser extension or a script that directly manipulates the DOM) ` + + `has moved or removed a node that Angular is still managing.`, ); } targetParent.insertBefore(newChild, refChild); @@ -555,13 +555,6 @@ function isTemplateNode(node: any): node is HTMLTemplateElement { return node.tagName === 'TEMPLATE' && node.content !== undefined; } -// Short description of a node for error messages. Truncates text so a huge text node can't blow -// up the message, same idea as `shorten()` in core's `hydration/error_handling.ts`. -function describeDomNode(node: Node): string { - const textContent = node.textContent?.slice(0, 50); - return textContent ? `${node.nodeName} ("${textContent}")` : node.nodeName; -} - class ShadowDomRenderer extends DefaultDomRenderer2 { private shadowRoot: any; diff --git a/packages/platform-server/test/full_app_hydration_spec.ts b/packages/platform-server/test/full_app_hydration_spec.ts index ca0c1c58924..b5ca863cd0f 100644 --- a/packages/platform-server/test/full_app_hydration_spec.ts +++ b/packages/platform-server/test/full_app_hydration_spec.ts @@ -6313,12 +6313,11 @@ describe('platform-server full application hydration integration', () => { } catch (e: unknown) { const error = e as Error; // This is the fixed behavior: a coded NG0500 RuntimeError, not a raw - // TypeError. Unlike other production RuntimeErrors, this one keeps a - // minimal message (the mismatched node's `nodeName` and `textContent`) - // rather than an empty one, since it's cheap (no DOM-printing machinery) - // and helps debugging a hydration mismatch that's otherwise invisible in prod. + // TypeError. Production intentionally gets a bare code with no + // description — building one (even a cheap one) is debug-only work + // that shouldn't ship unconditionally in production bundles. expect(error instanceof TypeError).toBe(false); - expect(error.message).toBe('NG0500: #text ("Not an element anymore!")'); + expect(error.message).toBe('NG0500'); expect(error.message).not.toContain('hasAttribute is not a function'); } finally { (globalThis as any).ngDevMode = previousNgDevMode; @@ -6393,6 +6392,148 @@ describe('platform-server full application hydration integration', () => { }, ); + it( + 'should throw a coded RuntimeError, not a raw TypeError, when siblingAfter runs out ' + + 'of DOM siblings to skip in production mode (ngDevMode off)', + async () => { + // Regression test. `siblingAfter` walks forward a fixed number of DOM siblings + // (based on the server-serialized container size) to find a container's anchor + // comment node. The dev-mode check that would normally catch a missing sibling + // (`validateSiblingNodeExists`) is gated behind `ngDevMode`, so it's removed from + // production builds. If the client DOM has fewer real siblings than the server + // expected (here, two of the three `@for`-rendered items are removed before + // hydration runs), production keeps walking past the end of the DOM and throws a + // raw TypeError instead of a coded RuntimeError. + @Component({ + selector: 'app', + template: ` +
+ @for (item of items; track item) { +

{{ item }}

+ } +
+ `, + }) + class SimpleComponent { + items = [1, 2, 3]; + private doc = inject(DOCUMENT); + private isServer = isPlatformServer(inject(PLATFORM_ID)); + ngAfterViewInit() { + // Only change the DOM on the server, right before it gets serialized. The + // serialized container size still says "3 items", but the DOM hydration + // actually sees will only have 1. + if (this.isServer) { + const items = this.doc.querySelectorAll('#abc p'); + items[1]?.remove(); + items[2]?.remove(); + } + } + } + + const html = await ssr(SimpleComponent); + const ssrContents = getAppContents(html); + + expect(ssrContents).toContain(' { + // Regression test for getParentRElement() (node_manipulation.ts) receiving a null + // TNode. Unlike the siblingAfter/navigateToNode bugs above, this one isn't gated + // behind ngDevMode at all in the original code — it reproduces every time, with no + // need to simulate a production build. + // + // The mechanism: an @if/@switch branch's content is its own embedded template, with + // its own TView, built lazily the first time that branch is actually rendered + // (renderView() in render.ts). If an error is thrown partway through that *first* + // pass — here, a hydration node-mismatch on the branch's *second* child, so the + // first child's TNode is created but the second's never is — TView.firstCreatePass + // still gets flipped to false in render.ts's `catch` block before the error + // propagates, and the TView is marked `incompleteFirstPass`. Unlike a *component's* + // TView (which gets rebuilt from scratch next time via + // getOrCreateComponentTView()'s `incompleteFirstPass` check), nothing rebuilds an + // *embedded view's* TView. So the next time this exact branch is selected again — + // here, by flipping the condition off and back on — its instructions see + // `!tView.firstCreatePass` and read straight from `tView.data[slot]` instead of + // creating a fresh TNode, and the second child's slot is still null from the + // interrupted first pass. + let instance!: SimpleComponent; + + @Component({ + selector: 'app', + template: ` + @if (cond()) { + first + {{ text() }} + } @else { + else + } + `, + }) + class SimpleComponent { + cond = signal(true); + text = signal('orig'); + private doc = inject(DOCUMENT); + constructor() { + instance = this; + } + ngAfterViewInit() { + // Swap the second for a
, bypassing Angular's own node tracking + // (mimics a third-party script mutating the DOM, like the test above), so + // hydration hits a tag mismatch on the branch's SECOND node specifically, + // interrupting its first creation pass after the first node already succeeded. + const spans = this.doc.querySelectorAll('app span'); + if (spans.length >= 2) { + const bad = this.doc.createElement('div'); + bad.textContent = spans[1].textContent; + spans[1].replaceWith(bad); + } + } + } + + const html = await ssr(SimpleComponent); + resetTViewsFor(SimpleComponent); + + const appRef = await prepareEnvironmentAndHydrate(doc, html, SimpleComponent, { + envProviders: [withNoopErrorHandler()], + }); + + // Re-enter the same @if branch: leave it, then come back. This is what exposes the + // corrupted TView from the interrupted first pass above. + instance.cond.set(false); + appRef.tick(); + instance.cond.set(true); + + expect(() => appRef.tick()).toThrowError(/NG0510/); + }, + ); + it('should if there are any third-party scripts that manipulate the DOM', async () => { @Component({ selector: 'app',