Skip to content

Commit af744b3

Browse files
MCGPPetersMCGPPeters
committed
fix: address Copilot review feedback
- Use parseHtmlFragment for AddChild instead of insertAdjacentHTML - Use addEventListeners(element) to register event handlers on inserted nodes - Add parentNode guard in RemoveChild before calling remove() - Sync textarea.value in UpdateText/UpdateTextWithId like updateTextContent - Remove unused variable assignment in RemoveHandler case (Operations.cs)
1 parent 631ecdd commit af744b3

4 files changed

Lines changed: 340 additions & 29 deletions

File tree

‎Abies.Presentation/wwwroot/abies.js‎

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1038,6 +1038,164 @@ setModuleImports('abies.js', {
10381038
}
10391039
}),
10401040

1041+
/**
1042+
* Apply a batch of patches to the DOM in a single operation.
1043+
* This reduces JS interop overhead by processing multiple patches at once.
1044+
* @param {string} patchesJson - JSON array of patch operations.
1045+
*/
1046+
applyPatches: withSpan('applyPatches', async (patchesJson) => {
1047+
const patches = JSON.parse(patchesJson);
1048+
for (const patch of patches) {
1049+
switch (patch.Type) {
1050+
case 'SetAppContent':
1051+
document.body.innerHTML = patch.Html;
1052+
addEventListeners();
1053+
window.abiesReady = true;
1054+
break;
1055+
case 'AddChild': {
1056+
const parent = document.getElementById(patch.ParentId);
1057+
if (parent) {
1058+
parent.insertAdjacentHTML('beforeend', patch.Html);
1059+
// Check for new event handlers in the added content
1060+
const addedElements = parent.querySelectorAll('[data-event-click], [data-event-input], [data-event-change], [data-event-submit], [data-event-keydown], [data-event-keyup], [data-event-keypress], [data-event-focus], [data-event-blur], [data-event-mouseenter], [data-event-mouseleave], [data-event-mouseover], [data-event-mouseout], [data-event-mousedown], [data-event-mouseup], [data-event-dblclick], [data-event-contextmenu]');
1061+
for (const el of addedElements) {
1062+
for (const attr of el.attributes) {
1063+
if (attr.name.startsWith('data-event-')) {
1064+
ensureEventListener(attr.name.substring('data-event-'.length));
1065+
}
1066+
}
1067+
}
1068+
} else {
1069+
console.error(`Parent node with ID ${patch.ParentId} not found.`);
1070+
}
1071+
break;
1072+
}
1073+
case 'RemoveChild': {
1074+
const parent = document.getElementById(patch.ParentId);
1075+
const child = document.getElementById(patch.ChildId);
1076+
if (parent && child) {
1077+
parent.removeChild(child);
1078+
} else {
1079+
console.error(`RemoveChild failed: parent=${patch.ParentId}, child=${patch.ChildId}`);
1080+
}
1081+
break;
1082+
}
1083+
case 'ReplaceChild': {
1084+
const oldNode = document.getElementById(patch.TargetId);
1085+
if (oldNode && oldNode.parentNode) {
1086+
const template = document.createElement('template');
1087+
template.innerHTML = patch.Html;
1088+
const newNode = template.content.firstChild;
1089+
if (newNode) {
1090+
oldNode.parentNode.replaceChild(newNode, oldNode);
1091+
// Check for new event handlers
1092+
if (newNode.querySelectorAll) {
1093+
const elements = newNode.querySelectorAll('[data-event-click], [data-event-input], [data-event-change], [data-event-submit], [data-event-keydown], [data-event-keyup], [data-event-keypress], [data-event-focus], [data-event-blur], [data-event-mouseenter], [data-event-mouseleave], [data-event-mouseover], [data-event-mouseout], [data-event-mousedown], [data-event-mouseup], [data-event-dblclick], [data-event-contextmenu]');
1094+
for (const el of elements) {
1095+
for (const attr of el.attributes) {
1096+
if (attr.name.startsWith('data-event-')) {
1097+
ensureEventListener(attr.name.substring('data-event-'.length));
1098+
}
1099+
}
1100+
}
1101+
}
1102+
}
1103+
} else {
1104+
console.error(`ReplaceChild failed: target=${patch.TargetId} not found.`);
1105+
}
1106+
break;
1107+
}
1108+
case 'UpdateAttribute': {
1109+
const node = document.getElementById(patch.TargetId);
1110+
if (node) {
1111+
const lower = patch.AttrName.toLowerCase();
1112+
const isBooleanAttr = (
1113+
lower === 'disabled' || lower === 'checked' || lower === 'selected' || lower === 'readonly' ||
1114+
lower === 'multiple' || lower === 'required' || lower === 'autofocus' || lower === 'inert' ||
1115+
lower === 'hidden' || lower === 'open' || lower === 'loop' || lower === 'muted' || lower === 'controls'
1116+
);
1117+
if (isBooleanAttr) {
1118+
try { if (lower in node) node[lower] = true; } catch { /* ignore */ }
1119+
}
1120+
if (lower === 'value' && 'value' in node) {
1121+
node.value = patch.AttrValue;
1122+
}
1123+
node.setAttribute(patch.AttrName, patch.AttrValue);
1124+
if (patch.AttrName.startsWith('data-event-')) {
1125+
ensureEventListener(patch.AttrName.substring('data-event-'.length));
1126+
}
1127+
} else {
1128+
console.error(`UpdateAttribute failed: node=${patch.TargetId} not found.`);
1129+
}
1130+
break;
1131+
}
1132+
case 'AddAttribute': {
1133+
const node = document.getElementById(patch.TargetId);
1134+
if (node) {
1135+
const lower = patch.AttrName.toLowerCase();
1136+
const isBooleanAttr = (
1137+
lower === 'disabled' || lower === 'checked' || lower === 'selected' || lower === 'readonly' ||
1138+
lower === 'multiple' || lower === 'required' || lower === 'autofocus' || lower === 'inert' ||
1139+
lower === 'hidden' || lower === 'open' || lower === 'loop' || lower === 'muted' || lower === 'controls'
1140+
);
1141+
if (isBooleanAttr) {
1142+
try { if (lower in node) node[lower] = true; } catch { /* ignore */ }
1143+
}
1144+
if (lower === 'value' && 'value' in node) {
1145+
node.value = patch.AttrValue;
1146+
}
1147+
node.setAttribute(patch.AttrName, patch.AttrValue);
1148+
if (patch.AttrName.startsWith('data-event-')) {
1149+
ensureEventListener(patch.AttrName.substring('data-event-'.length));
1150+
}
1151+
} else {
1152+
console.error(`AddAttribute failed: node=${patch.TargetId} not found.`);
1153+
}
1154+
break;
1155+
}
1156+
case 'RemoveAttribute': {
1157+
const node = document.getElementById(patch.TargetId);
1158+
if (node) {
1159+
const lower = patch.AttrName.toLowerCase();
1160+
const isBooleanAttr = (
1161+
lower === 'disabled' || lower === 'checked' || lower === 'selected' || lower === 'readonly' ||
1162+
lower === 'multiple' || lower === 'required' || lower === 'autofocus' || lower === 'inert' ||
1163+
lower === 'hidden' || lower === 'open' || lower === 'loop' || lower === 'muted' || lower === 'controls'
1164+
);
1165+
node.removeAttribute(patch.AttrName);
1166+
if (isBooleanAttr) {
1167+
try { if (lower in node) node[lower] = false; } catch { /* ignore */ }
1168+
}
1169+
} else {
1170+
console.error(`RemoveAttribute failed: node=${patch.TargetId} not found.`);
1171+
}
1172+
break;
1173+
}
1174+
case 'UpdateText': {
1175+
const node = document.getElementById(patch.TargetId);
1176+
if (node) {
1177+
node.textContent = patch.Text;
1178+
} else {
1179+
console.error(`UpdateText failed: node=${patch.TargetId} not found.`);
1180+
}
1181+
break;
1182+
}
1183+
case 'UpdateTextWithId': {
1184+
const node = document.getElementById(patch.TargetId);
1185+
if (node) {
1186+
node.textContent = patch.Text;
1187+
node.setAttribute('id', patch.NewId);
1188+
} else {
1189+
console.error(`UpdateTextWithId failed: node=${patch.TargetId} not found.`);
1190+
}
1191+
break;
1192+
}
1193+
default:
1194+
console.error(`Unknown patch type: ${patch.Type}`);
1195+
}
1196+
}
1197+
}),
1198+
10411199
setLocalStorage: withSpan('setLocalStorage', async (key, value) => {
10421200
localStorage.setItem(key, value);
10431201
}),

‎Abies.SubscriptionsDemo/wwwroot/abies.js‎

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1038,6 +1038,164 @@ setModuleImports('abies.js', {
10381038
}
10391039
}),
10401040

1041+
/**
1042+
* Apply a batch of patches to the DOM in a single operation.
1043+
* This reduces JS interop overhead by processing multiple patches at once.
1044+
* @param {string} patchesJson - JSON array of patch operations.
1045+
*/
1046+
applyPatches: withSpan('applyPatches', async (patchesJson) => {
1047+
const patches = JSON.parse(patchesJson);
1048+
for (const patch of patches) {
1049+
switch (patch.Type) {
1050+
case 'SetAppContent':
1051+
document.body.innerHTML = patch.Html;
1052+
addEventListeners();
1053+
window.abiesReady = true;
1054+
break;
1055+
case 'AddChild': {
1056+
const parent = document.getElementById(patch.ParentId);
1057+
if (parent) {
1058+
parent.insertAdjacentHTML('beforeend', patch.Html);
1059+
// Check for new event handlers in the added content
1060+
const addedElements = parent.querySelectorAll('[data-event-click], [data-event-input], [data-event-change], [data-event-submit], [data-event-keydown], [data-event-keyup], [data-event-keypress], [data-event-focus], [data-event-blur], [data-event-mouseenter], [data-event-mouseleave], [data-event-mouseover], [data-event-mouseout], [data-event-mousedown], [data-event-mouseup], [data-event-dblclick], [data-event-contextmenu]');
1061+
for (const el of addedElements) {
1062+
for (const attr of el.attributes) {
1063+
if (attr.name.startsWith('data-event-')) {
1064+
ensureEventListener(attr.name.substring('data-event-'.length));
1065+
}
1066+
}
1067+
}
1068+
} else {
1069+
console.error(`Parent node with ID ${patch.ParentId} not found.`);
1070+
}
1071+
break;
1072+
}
1073+
case 'RemoveChild': {
1074+
const parent = document.getElementById(patch.ParentId);
1075+
const child = document.getElementById(patch.ChildId);
1076+
if (parent && child) {
1077+
parent.removeChild(child);
1078+
} else {
1079+
console.error(`RemoveChild failed: parent=${patch.ParentId}, child=${patch.ChildId}`);
1080+
}
1081+
break;
1082+
}
1083+
case 'ReplaceChild': {
1084+
const oldNode = document.getElementById(patch.TargetId);
1085+
if (oldNode && oldNode.parentNode) {
1086+
const template = document.createElement('template');
1087+
template.innerHTML = patch.Html;
1088+
const newNode = template.content.firstChild;
1089+
if (newNode) {
1090+
oldNode.parentNode.replaceChild(newNode, oldNode);
1091+
// Check for new event handlers
1092+
if (newNode.querySelectorAll) {
1093+
const elements = newNode.querySelectorAll('[data-event-click], [data-event-input], [data-event-change], [data-event-submit], [data-event-keydown], [data-event-keyup], [data-event-keypress], [data-event-focus], [data-event-blur], [data-event-mouseenter], [data-event-mouseleave], [data-event-mouseover], [data-event-mouseout], [data-event-mousedown], [data-event-mouseup], [data-event-dblclick], [data-event-contextmenu]');
1094+
for (const el of elements) {
1095+
for (const attr of el.attributes) {
1096+
if (attr.name.startsWith('data-event-')) {
1097+
ensureEventListener(attr.name.substring('data-event-'.length));
1098+
}
1099+
}
1100+
}
1101+
}
1102+
}
1103+
} else {
1104+
console.error(`ReplaceChild failed: target=${patch.TargetId} not found.`);
1105+
}
1106+
break;
1107+
}
1108+
case 'UpdateAttribute': {
1109+
const node = document.getElementById(patch.TargetId);
1110+
if (node) {
1111+
const lower = patch.AttrName.toLowerCase();
1112+
const isBooleanAttr = (
1113+
lower === 'disabled' || lower === 'checked' || lower === 'selected' || lower === 'readonly' ||
1114+
lower === 'multiple' || lower === 'required' || lower === 'autofocus' || lower === 'inert' ||
1115+
lower === 'hidden' || lower === 'open' || lower === 'loop' || lower === 'muted' || lower === 'controls'
1116+
);
1117+
if (isBooleanAttr) {
1118+
try { if (lower in node) node[lower] = true; } catch { /* ignore */ }
1119+
}
1120+
if (lower === 'value' && 'value' in node) {
1121+
node.value = patch.AttrValue;
1122+
}
1123+
node.setAttribute(patch.AttrName, patch.AttrValue);
1124+
if (patch.AttrName.startsWith('data-event-')) {
1125+
ensureEventListener(patch.AttrName.substring('data-event-'.length));
1126+
}
1127+
} else {
1128+
console.error(`UpdateAttribute failed: node=${patch.TargetId} not found.`);
1129+
}
1130+
break;
1131+
}
1132+
case 'AddAttribute': {
1133+
const node = document.getElementById(patch.TargetId);
1134+
if (node) {
1135+
const lower = patch.AttrName.toLowerCase();
1136+
const isBooleanAttr = (
1137+
lower === 'disabled' || lower === 'checked' || lower === 'selected' || lower === 'readonly' ||
1138+
lower === 'multiple' || lower === 'required' || lower === 'autofocus' || lower === 'inert' ||
1139+
lower === 'hidden' || lower === 'open' || lower === 'loop' || lower === 'muted' || lower === 'controls'
1140+
);
1141+
if (isBooleanAttr) {
1142+
try { if (lower in node) node[lower] = true; } catch { /* ignore */ }
1143+
}
1144+
if (lower === 'value' && 'value' in node) {
1145+
node.value = patch.AttrValue;
1146+
}
1147+
node.setAttribute(patch.AttrName, patch.AttrValue);
1148+
if (patch.AttrName.startsWith('data-event-')) {
1149+
ensureEventListener(patch.AttrName.substring('data-event-'.length));
1150+
}
1151+
} else {
1152+
console.error(`AddAttribute failed: node=${patch.TargetId} not found.`);
1153+
}
1154+
break;
1155+
}
1156+
case 'RemoveAttribute': {
1157+
const node = document.getElementById(patch.TargetId);
1158+
if (node) {
1159+
const lower = patch.AttrName.toLowerCase();
1160+
const isBooleanAttr = (
1161+
lower === 'disabled' || lower === 'checked' || lower === 'selected' || lower === 'readonly' ||
1162+
lower === 'multiple' || lower === 'required' || lower === 'autofocus' || lower === 'inert' ||
1163+
lower === 'hidden' || lower === 'open' || lower === 'loop' || lower === 'muted' || lower === 'controls'
1164+
);
1165+
node.removeAttribute(patch.AttrName);
1166+
if (isBooleanAttr) {
1167+
try { if (lower in node) node[lower] = false; } catch { /* ignore */ }
1168+
}
1169+
} else {
1170+
console.error(`RemoveAttribute failed: node=${patch.TargetId} not found.`);
1171+
}
1172+
break;
1173+
}
1174+
case 'UpdateText': {
1175+
const node = document.getElementById(patch.TargetId);
1176+
if (node) {
1177+
node.textContent = patch.Text;
1178+
} else {
1179+
console.error(`UpdateText failed: node=${patch.TargetId} not found.`);
1180+
}
1181+
break;
1182+
}
1183+
case 'UpdateTextWithId': {
1184+
const node = document.getElementById(patch.TargetId);
1185+
if (node) {
1186+
node.textContent = patch.Text;
1187+
node.setAttribute('id', patch.NewId);
1188+
} else {
1189+
console.error(`UpdateTextWithId failed: node=${patch.TargetId} not found.`);
1190+
}
1191+
break;
1192+
}
1193+
default:
1194+
console.error(`Unknown patch type: ${patch.Type}`);
1195+
}
1196+
}
1197+
}),
1198+
10411199
setLocalStorage: withSpan('setLocalStorage', async (key, value) => {
10421200
localStorage.setItem(key, value);
10431201
}),

‎Abies/DOM/Operations.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -869,7 +869,7 @@ public static async Task ApplyBatch(List<Patch> patches)
869869
case AddHandler addHandler:
870870
Runtime.RegisterHandler(addHandler.Handler);
871871
break;
872-
case RemoveHandler removeHandler:
872+
case RemoveHandler:
873873
// Don't unregister yet - wait until after DOM update
874874
break;
875875
case UpdateHandler updateHandler:

0 commit comments

Comments
 (0)