Skip to content

Commit 14290a9

Browse files
janechuCopilot
andcommitted
fix: address seventh round of PR review feedback
- Fix MIGRATION.md: correct old API names in Removed column (parseRepeatStartMarker not parseRepeatViewStartMarker, parseElementBoundaryStartMarker(content) not (node)) - Upgrade targetContentBinding errors from plain Error to HydrationTargetElementError with factories and node context - Fix scanStop boundary: use previousSibling of boundary.first so the boundary node itself is still eligible for matching as a start marker - Remove unused isShadowRoot helper function Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent cd0a21b commit 14290a9

3 files changed

Lines changed: 22 additions & 18 deletions

File tree

packages/fast-element/MIGRATION.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,10 +71,10 @@ The hydration markers embedded in SSR output have been simplified from verbose,
7171
| `HydrationMarkup.isContentBindingStartMarker(data)` | `HydrationMarkup.isContentBindingStartMarker(data)` (unchanged signature, new implementation) |
7272
| `HydrationMarkup.isContentBindingEndMarker(data)` | `HydrationMarkup.isContentBindingEndMarker(data)` (unchanged signature, new implementation) |
7373
| `HydrationMarkup.parseAttributeBinding(element)` | `HydrationMarkup.parseAttributeBindingCount(element)` |
74-
| `HydrationMarkup.parseRepeatViewStartMarker(data)` | `HydrationMarkup.isRepeatViewStartMarker(data)` |
75-
| `HydrationMarkup.parseRepeatViewEndMarker(data)` | `HydrationMarkup.isRepeatViewEndMarker(data)` |
76-
| `HydrationMarkup.parseElementBoundaryStartMarker(node)` | `HydrationMarkup.isElementBoundaryStartMarker(node)` |
77-
| `HydrationMarkup.parseElementBoundaryEndMarker(node)` | `HydrationMarkup.isElementBoundaryEndMarker(node)` |
74+
| `HydrationMarkup.parseRepeatStartMarker(data)` | `HydrationMarkup.isRepeatViewStartMarker(data)` |
75+
| `HydrationMarkup.parseRepeatEndMarker(data)` | `HydrationMarkup.isRepeatViewEndMarker(data)` |
76+
| `HydrationMarkup.parseElementBoundaryStartMarker(content)` | `HydrationMarkup.isElementBoundaryStartMarker(node)` |
77+
| `HydrationMarkup.parseElementBoundaryEndMarker(content)` | `HydrationMarkup.isElementBoundaryEndMarker(node)` |
7878

7979
### Impact
8080

packages/fast-element/src/hydration/target-builder.ts

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,6 @@ export function createRangeForNodes(first: Node, last: Node): Range {
7474
return range;
7575
}
7676

77-
function isShadowRoot(node: Node): node is ShadowRoot {
78-
return node instanceof DocumentFragment && "mode" in node;
79-
}
80-
8177
/**
8278
* Maps compiled ViewBehaviorFactory IDs to their corresponding DOM nodes in the
8379
* server-rendered shadow root. Uses a TreeWalker to scan the existing DOM between
@@ -179,6 +175,7 @@ export function buildViewBindingTargets(
179175
node as Comment,
180176
walker,
181177
factory,
178+
factories,
182179
targets,
183180
boundaries,
184181
);
@@ -198,6 +195,7 @@ function targetContentBinding(
198195
node: Comment,
199196
walker: TreeWalker,
200197
factory: CompiledViewBehaviorFactory,
198+
factories: CompiledViewBehaviorFactory[],
201199
targets: ViewBehaviorTargets,
202200
boundaries: ViewBehaviorBoundaries,
203201
) {
@@ -206,11 +204,12 @@ function targetContentBinding(
206204
node.data = "";
207205

208206
if (current === null) {
209-
const root = node.getRootNode();
210-
throw new Error(
211-
`Error hydrating Comment node inside "${
212-
isShadowRoot(root) ? root.host.nodeName : root.nodeName
207+
throw new HydrationTargetElementError(
208+
`Error hydrating content binding inside "${
209+
(node.getRootNode() as ShadowRoot).host?.nodeName ?? "unknown"
213210
}": no sibling found after content binding start marker.`,
211+
factories,
212+
node,
214213
);
215214
}
216215

@@ -232,11 +231,12 @@ function targetContentBinding(
232231
}
233232

234233
if (current === null) {
235-
const root = node.getRootNode();
236-
throw new Error(
237-
`Error hydrating Comment node inside "${
238-
isShadowRoot(root) ? root.host.nodeName : root.nodeName
239-
}".`,
234+
throw new HydrationTargetElementError(
235+
`Error hydrating content binding inside "${
236+
(node.getRootNode() as ShadowRoot).host?.nodeName ?? "unknown"
237+
}": missing fe:/b end marker.`,
238+
factories,
239+
node,
240240
);
241241
}
242242

packages/fast-element/src/templating/repeat.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -415,10 +415,14 @@ export class RepeatBehavior<TSource = any> implements ViewBehavior, Subscriber {
415415
// Determine the scan boundary for this repeat directive.
416416
// Use the content binding boundaries (from the parent's fe:b/fe:/b
417417
// markers) to avoid scanning into sibling repeat blocks.
418+
// Set scanStop to the node BEFORE the boundary's first node so the
419+
// boundary node itself is still eligible for matching as a start marker.
418420
const boundaries =
419421
isHydratable(this.controller) &&
420422
this.controller.bindingViewBoundaries[this.directive.targetNodeId];
421-
const scanStop: Node | null = boundaries ? boundaries.first : null;
423+
const scanStop: Node | null = boundaries
424+
? (boundaries.first.previousSibling ?? null)
425+
: null;
422426

423427
let current = this.location.previousSibling;
424428
let itemIndex = itemCount - 1; // items render in order; walk backward

0 commit comments

Comments
 (0)