Skip to content

Commit f561e23

Browse files
committed
ENG-1470: Fix dual-read gaps found during flag-ON validation
Bootstrap legacy config blocks in initSchema so the plugin works on fresh graphs without createConfigObserver/configPageTabs: - trigger, grammar/relations, export, Suggestive Mode, Left Sidebar - Reuses existing ensureBlocksExist/buildBlockMap helpers + DEFAULT_RELATION_VALUES Fix duplicate block accumulation bugs: - Page Groups: getSubTree auto-create race (ensureLegacyConfigBlocks pre-creates) - Folded: lookup-based delete via getBasicTreeByParentUid instead of stale uid - scratch/enabled: switched getSubTree({ parentUid }) to tree-based reads - Folded in convertToComplexSection: removed erroneous block creation Fix dual-read comparison: - Replace JSON.stringify with deepEqual (handles key order, undefined/empty/false) - Deterministic async ordering: await legacy write → refreshConfigTree → blockProp write (BlockPropSettingPanels, LeftSidebarView toggleFoldedState, AdminPanel suggestive mode) - Use getPersonalSettings() (raw read) in toggleFoldedState to avoid mid-write comparison Fix storedRelations import path (renderNodeConfigPage → data/constants)
1 parent 26fef42 commit f561e23

7 files changed

Lines changed: 154 additions & 49 deletions

File tree

apps/roam/src/components/LeftSidebarView.tsx

Lines changed: 29 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import { getLeftSidebarSettings } from "~/utils/getLeftSidebarSettings";
3636
import {
3737
getGlobalSetting,
3838
getPersonalSetting,
39+
getPersonalSettings,
3940
setGlobalSetting,
4041
setPersonalSetting,
4142
} from "~/components/settings/utils/accessors";
@@ -45,10 +46,7 @@ import {
4546
LEFT_SIDEBAR_KEYS,
4647
LEFT_SIDEBAR_SETTINGS_KEYS,
4748
} from "~/components/settings/utils/settingKeys";
48-
import type {
49-
LeftSidebarGlobalSettings,
50-
PersonalSection,
51-
} from "~/components/settings/utils/zodSchema";
49+
import type { LeftSidebarGlobalSettings } from "~/components/settings/utils/zodSchema";
5250
import { createBlock } from "roamjs-components/writes";
5351
import deleteBlock from "roamjs-components/writes/deleteBlock";
5452
import getTextByBlockUid from "roamjs-components/queries/getTextByBlockUid";
@@ -112,7 +110,7 @@ const openTarget = async (e: React.MouseEvent, targetUid: string) => {
112110
}
113111
};
114112

115-
const toggleFoldedState = ({
113+
const toggleFoldedState = async ({
116114
isOpen,
117115
setIsOpen,
118116
folded,
@@ -130,23 +128,26 @@ const toggleFoldedState = ({
130128
const newFolded = !isOpen;
131129

132130
if (isOpen) {
133-
setIsOpen(false);
134-
if (folded.uid) {
135-
void deleteBlock(folded.uid);
136-
folded.uid = undefined;
137-
folded.value = false;
138-
}
131+
const children = getBasicTreeByParentUid(parentUid);
132+
await Promise.all(
133+
children
134+
.filter((c) => c.text === "Folded")
135+
.map((c) => deleteBlock(c.uid)),
136+
);
137+
folded.uid = undefined;
138+
folded.value = false;
139139
} else {
140-
setIsOpen(true);
141140
const newUid = window.roamAlphaAPI.util.generateUID();
142-
void createBlock({
141+
await createBlock({
143142
parentUid,
144143
node: { text: "Folded", uid: newUid },
145144
});
146145
folded.uid = newUid;
147146
folded.value = true;
148147
}
149148

149+
refreshConfigTree();
150+
150151
if (isGlobal) {
151152
setGlobalSetting(
152153
[
@@ -157,13 +158,20 @@ const toggleFoldedState = ({
157158
newFolded,
158159
);
159160
} else if (sectionIndex !== undefined) {
160-
const sections =
161-
getPersonalSetting<PersonalSection[]>([PERSONAL_KEYS.leftSidebar]) || [];
161+
const sections = [...getPersonalSettings()[PERSONAL_KEYS.leftSidebar]];
162162
if (sections[sectionIndex]) {
163-
sections[sectionIndex].Settings.Folded = newFolded;
163+
sections[sectionIndex] = {
164+
...sections[sectionIndex],
165+
Settings: {
166+
...sections[sectionIndex].Settings,
167+
Folded: newFolded,
168+
},
169+
};
164170
setPersonalSetting([PERSONAL_KEYS.leftSidebar], sections);
165171
}
166172
}
173+
174+
setIsOpen(newFolded);
167175
};
168176

169177
const SectionChildren = ({
@@ -225,7 +233,7 @@ const PersonalSectionItem = ({
225233
const handleChevronClick = () => {
226234
if (!section.settings) return;
227235

228-
toggleFoldedState({
236+
void toggleFoldedState({
229237
isOpen,
230238
setIsOpen,
231239
folded: section.settings.folded,
@@ -297,7 +305,7 @@ const GlobalSection = ({ config }: { config: LeftSidebarConfig["global"] }) => {
297305
className="sidebar-title-button flex w-full items-center border-none bg-transparent py-1 pl-6 pr-2.5 font-semibold outline-none"
298306
onClick={() => {
299307
if (!isCollapsable || !config.settings) return;
300-
toggleFoldedState({
308+
void toggleFoldedState({
301309
isOpen,
302310
setIsOpen,
303311
folded: config.settings.folded,
@@ -333,9 +341,9 @@ const buildConfig = (): LeftSidebarConfig => {
333341
const globalValues = getGlobalSetting<LeftSidebarGlobalSettings>([
334342
GLOBAL_KEYS.leftSidebar,
335343
]);
336-
const personalValues = getPersonalSetting<PersonalSection[]>([
337-
PERSONAL_KEYS.leftSidebar,
338-
]);
344+
const personalValues = getPersonalSetting<
345+
ReturnType<typeof getPersonalSettings>[typeof PERSONAL_KEYS.leftSidebar]
346+
>([PERSONAL_KEYS.leftSidebar]);
339347

340348
// Read UIDs from old system (needed for fold CRUD during dual-write)
341349
const oldConfig = getCurrentLeftSidebarConfig();

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

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -294,12 +294,15 @@ const FeatureFlagsTab = (): React.ReactElement => {
294294
if (checked) {
295295
setIsAlertOpen(true);
296296
} else {
297-
if (suggestiveModeUid) {
298-
void deleteBlock(suggestiveModeUid);
299-
setSuggestiveModeUid(undefined);
300-
}
301-
setSuggestiveModeEnabled(false);
302-
setFeatureFlag("Suggestive mode enabled", false);
297+
void (async () => {
298+
if (suggestiveModeUid) {
299+
await deleteBlock(suggestiveModeUid);
300+
setSuggestiveModeUid(undefined);
301+
}
302+
refreshConfigTree();
303+
setSuggestiveModeEnabled(false);
304+
setFeatureFlag("Suggestive mode enabled", false);
305+
})();
303306
}
304307
}}
305308
labelElement={
@@ -321,6 +324,7 @@ const FeatureFlagsTab = (): React.ReactElement => {
321324
node: { text: "(BETA) Suggestive Mode Enabled" },
322325
}).then((uid) => {
323326
setSuggestiveModeUid(uid);
327+
refreshConfigTree();
324328
setSuggestiveModeEnabled(true);
325329
setFeatureFlag("Suggestive mode enabled", true);
326330
setIsAlertOpen(false);

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

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -125,14 +125,9 @@ const SectionItem = memo(
125125
order: 0,
126126
node: { text: "Settings" },
127127
});
128-
const foldedUid = await createBlock({
129-
parentUid: settingsUid,
130-
order: 0,
131-
node: { text: "Folded" },
132-
});
133128
const truncateSettingUid = await createBlock({
134129
parentUid: settingsUid,
135-
order: 1,
130+
order: 0,
136131
node: { text: "Truncate-result?", children: [{ text: "75" }] },
137132
});
138133

@@ -149,7 +144,7 @@ const SectionItem = memo(
149144
...s,
150145
settings: {
151146
uid: settingsUid,
152-
folded: { uid: foldedUid, value: false },
147+
folded: { uid: undefined, value: false },
153148
truncateResult: { uid: truncateSettingUid, value: 75 },
154149
},
155150
childrenUid,
@@ -167,7 +162,7 @@ const SectionItem = memo(
167162
...s,
168163
settings: {
169164
uid: settingsUid,
170-
folded: { uid: foldedUid, value: false },
165+
folded: { uid: undefined, value: false },
171166
truncateResult: { uid: truncateSettingUid, value: 75 },
172167
},
173168
children: [],

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

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
import Description from "roamjs-components/components/Description";
1919
import useSingleChildValue from "roamjs-components/components/ConfigPanels/useSingleChildValue";
2020
import getShallowTreeByParentUid from "roamjs-components/queries/getShallowTreeByParentUid";
21+
import refreshConfigTree from "~/utils/refreshConfigTree";
2122
import {
2223
getGlobalSetting,
2324
getPersonalSetting,
@@ -143,8 +144,9 @@ const BaseTextPanel = ({
143144
window.clearTimeout(debounceRef.current);
144145
debounceRef.current = window.setTimeout(() => {
145146
if (errorRef.current) return;
146-
setter(settingKeys, newValue);
147147
syncToBlock?.(newValue);
148+
refreshConfigTree();
149+
setter(settingKeys, newValue);
148150
}, DEBOUNCE_MS);
149151
};
150152

@@ -227,8 +229,9 @@ const BaseFlagPanel = ({
227229
}
228230

229231
setInternalValue(checked);
230-
setter(settingKeys, checked);
231232
await syncFlagToBlock(checked);
233+
refreshConfigTree();
234+
setter(settingKeys, checked);
232235
onChange?.(checked);
233236
};
234237

@@ -276,8 +279,9 @@ const BaseNumberPanel = ({
276279
const handleChange = (valueAsNumber: number) => {
277280
if (Number.isNaN(valueAsNumber)) return;
278281
setValue(valueAsNumber);
279-
setter(settingKeys, valueAsNumber);
280282
syncToBlock?.(valueAsNumber);
283+
refreshConfigTree();
284+
setter(settingKeys, valueAsNumber);
281285
onChange?.(valueAsNumber);
282286
};
283287

@@ -323,8 +327,9 @@ const BaseSelectPanel = ({
323327
const handleChange = (e: ChangeEvent<HTMLSelectElement>) => {
324328
const newValue = e.target.value;
325329
setValue(newValue);
326-
setter(settingKeys, newValue);
327330
syncToBlock?.(newValue);
331+
refreshConfigTree();
332+
setter(settingKeys, newValue);
328333
};
329334

330335
return (
@@ -400,6 +405,7 @@ const BaseMultiTextPanel = ({
400405
},
401406
});
402407
childUidsRef.current = [...childUidsRef.current, valueUid];
408+
refreshConfigTree();
403409
}
404410
}
405411
};
@@ -408,7 +414,6 @@ const BaseMultiTextPanel = ({
408414
// eslint-disable-next-line @typescript-eslint/naming-convention
409415
const newValues = values.filter((_, i) => i !== index);
410416
setValues(newValues);
411-
setter(settingKeys, newValues);
412417
onChange?.(newValues);
413418

414419
if (hasBlockSync) {
@@ -420,7 +425,9 @@ const BaseMultiTextPanel = ({
420425
// eslint-disable-next-line @typescript-eslint/naming-convention
421426
(_, i) => i !== index,
422427
);
428+
refreshConfigTree();
423429
}
430+
setter(settingKeys, newValues);
424431
};
425432

426433
const handleKeyDown = (e: React.KeyboardEvent) => {

apps/roam/src/components/settings/utils/accessors.ts

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,24 @@ import { PERSONAL_KEYS, QUERY_KEYS, GLOBAL_KEYS } from "./settingKeys";
4545
const isRecord = (value: unknown): value is Record<string, unknown> =>
4646
typeof value === "object" && value !== null && !Array.isArray(value);
4747

48+
const deepEqual = (a: unknown, b: unknown): boolean => {
49+
if (a === b) return true;
50+
const isEmpty = (v: unknown) => v === undefined || v === "" || v === false;
51+
if (isEmpty(a) && isEmpty(b)) return true;
52+
if (a == null || b == null) return a === b;
53+
if (Array.isArray(a) && Array.isArray(b)) {
54+
if (a.length !== b.length) return false;
55+
return a.every((v, i) => deepEqual(v, b[i]));
56+
}
57+
if (isRecord(a) && isRecord(b)) {
58+
const keysA = Object.keys(a);
59+
const keysB = Object.keys(b);
60+
if (keysA.length !== keysB.length) return false;
61+
return keysA.every((k) => k in b && deepEqual(a[k], b[k]));
62+
}
63+
return false;
64+
};
65+
4866
const unwrapSchema = (schema: z.ZodTypeAny): z.ZodTypeAny => {
4967
let current = schema;
5068
let didUnwrap = true;
@@ -444,7 +462,10 @@ const getLegacyGlobalSetting = (keys: string[]): unknown => {
444462
};
445463

446464
const getLegacyQuerySettingByParentUid = (parentUid: string) => {
447-
const scratchNode = getSubTree({ parentUid, key: "scratch" });
465+
const scratchNode = getSubTree({
466+
tree: getBasicTreeByParentUid(parentUid),
467+
key: "scratch",
468+
});
448469
const conditionsNode = getSubTree({
449470
tree: scratchNode.children,
450471
key: "conditions",
@@ -514,6 +535,9 @@ const getLegacyDiscourseNodeSetting = (
514535
}).children;
515536
const indexUid = getSubTree({ tree, key: "Index" }).uid;
516537
const specificationUid = getSubTree({ tree, key: "Specification" }).uid;
538+
const specificationQuery = specificationUid
539+
? getLegacyQuerySettingByParentUid(specificationUid)
540+
: DEFAULT_LEGACY_QUERY;
517541

518542
const legacySettings = {
519543
type: nodeUid,
@@ -542,11 +566,11 @@ const getLegacyDiscourseNodeSetting = (
542566
: DEFAULT_LEGACY_QUERY,
543567
specification: {
544568
enabled: specificationUid
545-
? !!getSubTree({ parentUid: specificationUid, key: "enabled" }).uid
569+
? getBasicTreeByParentUid(specificationUid).some(
570+
(c) => c.text === "enabled",
571+
) || specificationQuery.conditions.length > 0
546572
: false,
547-
query: specificationUid
548-
? getLegacyQuerySettingByParentUid(specificationUid)
549-
: DEFAULT_LEGACY_QUERY,
573+
query: specificationQuery,
550574
},
551575
};
552576

@@ -803,7 +827,7 @@ export const getGlobalSetting = <T = unknown>(
803827
const settings = getGlobalSettings();
804828
const blockPropsValue = readPathValue(settings, keys);
805829
const legacyValue = getLegacyGlobalSetting(keys);
806-
if (JSON.stringify(blockPropsValue) !== JSON.stringify(legacyValue)) {
830+
if (!deepEqual(blockPropsValue, legacyValue)) {
807831
console.warn(
808832
`[DG Dual-Read] Mismatch at Global > ${formatSettingPath(keys)}`,
809833
{ blockProps: blockPropsValue, legacy: legacyValue },
@@ -875,7 +899,7 @@ export const getPersonalSetting = <T = unknown>(
875899
const settings = getPersonalSettings();
876900
const blockPropsValue = readPathValue(settings, keys);
877901
const legacyValue = getLegacyPersonalSetting(keys);
878-
if (JSON.stringify(blockPropsValue) !== JSON.stringify(legacyValue)) {
902+
if (!deepEqual(blockPropsValue, legacyValue)) {
879903
console.warn(
880904
`[DG Dual-Read] Mismatch at Personal > ${formatSettingPath(keys)}`,
881905
{ blockProps: blockPropsValue, legacy: legacyValue },
@@ -959,7 +983,7 @@ export const getDiscourseNodeSetting = <T = unknown>(
959983
const settings = getDiscourseNodeSettings(nodeType);
960984
const blockPropsValue = settings ? readPathValue(settings, keys) : undefined;
961985
const legacyValue = getLegacyDiscourseNodeSetting(nodeType, keys);
962-
if (JSON.stringify(blockPropsValue) !== JSON.stringify(legacyValue)) {
986+
if (!deepEqual(blockPropsValue, legacyValue)) {
963987
console.warn(
964988
`[DG Dual-Read] Mismatch at Discourse Node (${nodeType}) > ${formatSettingPath(keys)}`,
965989
{ blockProps: blockPropsValue, legacy: legacyValue },

0 commit comments

Comments
 (0)