Skip to content

Commit 9593e71

Browse files
authored
ENG-1716: Bug fix: Template does not render from block props (#1016)
* ENG-1716: Render template settings from block props When the new settings store flag is on, materialize the block-props template into an ephemeral Template-Block-props block as the last child of node.type, render that, and delete it on unmount. Flag-off behavior is unchanged. * ENG-1716: Dual-write template edits back to legacy Template block When the new settings store flag is on, edits in the buffer Template-Block-props block are mirrored to the legacy Template block via position-walked block.update calls. On length mismatch (only expected during testing/manual divergence) the mirror logs a warning and skips that subtree. * ENG-1716: Extend buffer->legacy mirror to handle add/delete Mirror now creates legacy children when buffer has extras and deletes legacy children when buffer has fewer, in addition to in-place block.update for matching positions. Replaces the prior length-match guard which caused dual-write to silently skip whenever the user added or removed template lines. * ENG-1716: Guard render effect against stale async continuation The ensureChildren promise resolves asynchronously, so its .then callback could run after the effect's cleanup fired - re-registering a pull watch on a stale renderUid and stomping pullWatchArgsRef. Add a cancelled flag (same pattern as the buffer-lifecycle effect) so the continuation aborts after teardown. * ENG-1716: Mirror heading and open alongside string The mirror only pushed block string to legacy, so a heading-level or collapse-state change in the buffer never propagated. Update the same block.update call to include heading and open whenever any of the three differ between buffer and legacy. Also drops uid from the buffer-creation effect's dependency array since the body no longer references it. * ENG-1716: Rename useNewStore -> isNewStore useNewStore reads like a React hook (use* convention) but is a boolean. Rename to isNewStore so the name matches the type.
1 parent e84cc2d commit 9593e71

1 file changed

Lines changed: 107 additions & 11 deletions

File tree

apps/roam/src/components/settings/components/EphemeralBlocksPanel.tsx

Lines changed: 107 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,20 @@
1-
import React, { useRef, useEffect, useCallback } from "react";
1+
import React, { useRef, useEffect, useCallback, useState } from "react";
22
import { Label } from "@blueprintjs/core";
33
import Description from "roamjs-components/components/Description";
44
import createBlock from "roamjs-components/writes/createBlock";
5+
import deleteBlock from "roamjs-components/writes/deleteBlock";
56
import getFullTreeByParentUid from "roamjs-components/queries/getFullTreeByParentUid";
67
import getFirstChildUidByBlockUid from "roamjs-components/queries/getFirstChildUidByBlockUid";
78
import type { InputTextNode, TreeNode } from "roamjs-components/types";
89
import type { RoamNodeType } from "~/components/settings/utils/zodSchema";
9-
import { setDiscourseNodeSetting } from "~/components/settings/utils/accessors";
10+
import {
11+
isNewSettingsStoreEnabled,
12+
setDiscourseNodeSetting,
13+
} from "~/components/settings/utils/accessors";
1014
import type { DiscourseNodeBaseProps } from "./BlockPropSettingPanels";
1115

1216
const DEBOUNCE_MS = 250;
17+
const TEMPLATE_BUFFER_TEXT = "Template-Block-props";
1318

1419
type DualWriteBlocksPanelProps = DiscourseNodeBaseProps & {
1520
uid: string;
@@ -28,6 +33,62 @@ const serializeBlockTree = (children: TreeNode[]): RoamNodeType[] =>
2833
}),
2934
}));
3035

36+
const treeNodeToInputTextNode = (node: TreeNode): InputTextNode => ({
37+
text: node.text,
38+
...(node.heading && { heading: node.heading as 0 | 1 | 2 | 3 }),
39+
...(node.open === false && { open: false }),
40+
...(node.children.length > 0 && {
41+
children: [...node.children]
42+
.sort((a, b) => a.order - b.order)
43+
.map(treeNodeToInputTextNode),
44+
}),
45+
});
46+
47+
const mirrorBufferToLegacyChildren = (
48+
bufferChildren: TreeNode[],
49+
legacyChildren: TreeNode[],
50+
legacyParentUid: string,
51+
): void => {
52+
const sortedBuffer = [...bufferChildren].sort((a, b) => a.order - b.order);
53+
const sortedLegacy = [...legacyChildren].sort((a, b) => a.order - b.order);
54+
const minLen = Math.min(sortedBuffer.length, sortedLegacy.length);
55+
56+
for (let i = 0; i < minLen; i++) {
57+
const bufferNode = sortedBuffer[i];
58+
const legacyNode = sortedLegacy[i];
59+
if (
60+
bufferNode.text !== legacyNode.text ||
61+
bufferNode.heading !== legacyNode.heading ||
62+
bufferNode.open !== legacyNode.open
63+
) {
64+
void window.roamAlphaAPI.data.block.update({
65+
block: {
66+
uid: legacyNode.uid,
67+
string: bufferNode.text,
68+
...(bufferNode.heading !== undefined && {
69+
heading: bufferNode.heading,
70+
}),
71+
...(bufferNode.open !== undefined && { open: bufferNode.open }),
72+
},
73+
});
74+
}
75+
mirrorBufferToLegacyChildren(
76+
bufferNode.children,
77+
legacyNode.children,
78+
legacyNode.uid,
79+
);
80+
}
81+
82+
for (let i = minLen; i < sortedBuffer.length; i++) {
83+
const node = treeNodeToInputTextNode(sortedBuffer[i]);
84+
void createBlock({ node, parentUid: legacyParentUid, order: i });
85+
}
86+
87+
for (let i = minLen; i < sortedLegacy.length; i++) {
88+
void deleteBlock(sortedLegacy[i].uid);
89+
}
90+
};
91+
3192
const DualWriteBlocksPanel = ({
3293
nodeType,
3394
settingKeys,
@@ -44,21 +105,51 @@ const DualWriteBlocksPanel = ({
44105
[string, string, (before: unknown, after: unknown) => void] | null
45106
>(null);
46107

108+
const isNewStore = isNewSettingsStoreEnabled();
109+
const [bufferUid, setBufferUid] = useState<string | null>(null);
110+
const renderUid = isNewStore ? bufferUid : uid;
111+
112+
useEffect(() => {
113+
if (!isNewStore || !nodeType) return;
114+
let cancelled = false;
115+
const newUid = window.roamAlphaAPI.util.generateUID();
116+
const dv = defaultValueRef.current;
117+
const seed: InputTextNode[] = dv && dv.length > 0 ? dv : [{ text: " " }];
118+
void createBlock({
119+
node: { text: TEMPLATE_BUFFER_TEXT, uid: newUid, children: seed },
120+
parentUid: nodeType,
121+
order: "last",
122+
}).then(() => {
123+
if (!cancelled) setBufferUid(newUid);
124+
});
125+
return () => {
126+
cancelled = true;
127+
setBufferUid(null);
128+
void deleteBlock(newUid);
129+
};
130+
}, [isNewStore, nodeType]);
131+
47132
const handleChange = useCallback(() => {
133+
if (!renderUid) return;
48134
window.clearTimeout(debounceRef.current);
49135
debounceRef.current = window.setTimeout(() => {
50-
const tree = getFullTreeByParentUid(uid);
136+
const tree = getFullTreeByParentUid(renderUid);
51137
const serialized = serializeBlockTree(tree.children);
52138
setDiscourseNodeSetting(nodeType, settingKeys, serialized);
139+
if (isNewStore && renderUid !== uid) {
140+
const legacyTree = getFullTreeByParentUid(uid);
141+
mirrorBufferToLegacyChildren(tree.children, legacyTree.children, uid);
142+
}
53143
}, DEBOUNCE_MS);
54-
}, [uid, nodeType, settingKeys]);
144+
}, [renderUid, uid, isNewStore, nodeType, settingKeys]);
55145

56146
useEffect(() => {
57147
const el = containerRef.current;
58-
if (!el || !uid) return;
148+
if (!el || !renderUid) return;
59149

150+
let cancelled = false;
60151
const pattern = "[:block/string :block/order {:block/children ...}]";
61-
const entityId = `[:block/uid "${uid}"]`;
152+
const entityId = `[:block/uid "${renderUid}"]`;
62153
const callback = () => handleChange();
63154

64155
const registerPullWatch = () => {
@@ -67,31 +158,36 @@ const DualWriteBlocksPanel = ({
67158
};
68159

69160
const dv = defaultValueRef.current;
70-
const ensureChildren = getFirstChildUidByBlockUid(uid)
161+
const ensureChildren = getFirstChildUidByBlockUid(renderUid)
71162
? Promise.resolve()
72163
: (dv && dv.length > 0
73164
? Promise.all(
74165
dv.map((node, i) =>
75-
createBlock({ node, parentUid: uid, order: i }),
166+
createBlock({ node, parentUid: renderUid, order: i }),
76167
),
77168
)
78-
: createBlock({ node: { text: " " }, parentUid: uid })
169+
: createBlock({ node: { text: " " }, parentUid: renderUid })
79170
).then(() => {});
80171

81172
void ensureChildren.then(() => {
173+
if (cancelled) return;
82174
el.innerHTML = "";
83-
void window.roamAlphaAPI.ui.components.renderBlock({ uid, el });
175+
void window.roamAlphaAPI.ui.components.renderBlock({
176+
uid: renderUid,
177+
el,
178+
});
84179
registerPullWatch();
85180
});
86181

87182
return () => {
183+
cancelled = true;
88184
window.clearTimeout(debounceRef.current);
89185
if (pullWatchArgsRef.current) {
90186
window.roamAlphaAPI.data.removePullWatch(...pullWatchArgsRef.current);
91187
pullWatchArgsRef.current = null;
92188
}
93189
};
94-
}, [uid, handleChange]);
190+
}, [renderUid, handleChange]);
95191

96192
return (
97193
<>

0 commit comments

Comments
 (0)