Skip to content

Commit d8ff16f

Browse files
authored
Eng 1616 add getconfigtree equivalent for block pros on init (#944)
* ENG-1616: Bulk-read settings + thread snapshot (with timing logs) Cut plugin load from ~20925ms to ~1327ms (94%) on a real graph by collapsing per-call settings accessors into a single bulk read at init and threading that snapshot through the init chain + observer callbacks. Key changes: - accessors.ts: bulkReadSettings() runs ONE pull query against the settings page's direct children and returns { featureFlags, globalSettings, personalSettings } parsed via Zod. readPathValue exported. - getDiscourseNodes / getDiscourseRelations / getAllRelations: optional snapshot param threaded through, no breaking changes to existing callers. - initializeDiscourseNodes + refreshConfigTree (+ registerDiscourseDatalog- Translators, getDiscourseRelationLabels): accept and forward snapshot. - index.ts: bulkReadSettings() at the top of init; snapshot threaded into initializeDiscourseNodes, refreshConfigTree, initObservers, installDiscourseFloatingMenu, setInitialQueryPages, and the 3 sync sites inside index.ts itself. - initializeObserversAndListeners.ts: snapshot threaded into the sync-init body; pageTitleObserver + leftSidebarObserver callbacks call bulkReadSettings() per fire (fresh, not stale); nodeTagPopupButtonObserver uses per-sync-batch memoization via queueMicrotask; hashChangeListener and nodeCreationPopoverListener use bulkReadSettings() per fire. - findDiscourseNode: snapshot param added; getDiscourseNodes() default-arg moved inside the cache-miss branch so cache hits don't waste the call. - isQueryPage / isCanvasPage / QueryPagesPanel.getQueryPages: optional snapshot param. - LeftSidebarView.buildConfig / useConfig / mountLeftSidebar: optional initialSnapshot threaded for the first render; emitter-driven updates keep using live reads for post-mount reactivity. - DiscourseFloatingMenu.installDiscourseFloatingMenu: optional snapshot. - posthog.initPostHog: removed redundant internal getPersonalSetting check (caller already guards from the snapshot). - migrateLegacyToBlockProps.hasGraphMigrationMarker: accepts the existing blockMap and does an O(1) lookup instead of a getBlockUidByTextOnPage scan. Includes per-phase timing console.logs across index.ts, refreshConfigTree, init.ts, initSettingsPageBlocks, and initObservers. Committed as a checkpoint so we can reference measurements later; will be removed in the next commit. * ENG-1616: Remove plugin-load timing logs Removes the per-phase console.log instrumentation added in the previous commit. All the [DG Plugin] / [DG Nav] logs and their `mark()` / `markPhase()` helpers are gone. Code behavior unchanged. Dropped in this commit: - index.ts: mark() closure, load start/done logs, and all phase marks. - initializeObserversAndListeners.ts: markPhase() closure, per-observer marks, pageTitleObserver fire log, hashChangeListener [DG Nav] logs. - LeftSidebarView.tsx: openTarget [DG Nav] click/resolve logs. - refreshConfigTree.ts: mark() closure and all phase marks. - init.ts: mark() closures in initSchema and initSettingsPageBlocks. - accessors.ts: bulkReadSettings internal timing log. - index.ts: unused getPluginElapsedTime import. Previous commit (343dc11) kept as a checkpoint for future drill-downs. * ENG-1616: Address review — typed indexing, restore dgDualReadLog, optional snapshot - index.ts: move initPluginTimer() back to its original position (after early-return checks) so timing isn't started for graphs that bail out. - Replace readPathValue + `as T | undefined` casts with direct typed indexing on the Zod-derived snapshot types across: - index.ts (disallowDiagnostics, isStreamlineStylingEnabled) - initializeObserversAndListeners.ts (suggestiveModeOverlay, pagePreview, discourseContextOverlay, globalTrigger, personalTriggerCombo, customTrigger) — also drops dead `?? "\\"` and `?? "@"` fallbacks since Zod defaults already populate them. - isCanvasPage.ts (canvasPageFormat) - setQueryPages.ts + QueryPagesPanel.tsx (nested [Query][Query pages]) - setQueryPages.setInitialQueryPages: snapshot is now optional with a getPersonalSetting fallback, matching the pattern used elsewhere (getQueryPages, isCanvasPage, etc.). - init.ts: restore logDualReadComparison + window.dgDualReadLog so the on-demand console helper is available again. NOT auto-called on init — invoke window.dgDualReadLog() manually to dump the comparison. * ENG-1616: Log total plugin load time Capture performance.now() at the top of runExtension and log the elapsed milliseconds just before the unload handler is wired, so we have a single broad measurement of plugin init cost on each load. * ENG-1616: Tighten init-only leaves to required snapshot, AGENTS.md compliance Make snapshot required at six init-only leaves where caller audit showed every site already passed one: installDiscourseFloatingMenu, initializeDiscourseNodes, setInitialQueryPages, isQueryPage, isCurrentPageCanvas, isSidebarCanvas. No cascade — only at the leaves. Drop dead fallback code that was reachable only via the optional path: - setQueryPages: legacy string|Record coercion ladder (snapshot is Zod-typed string[]) - DiscourseFloatingMenu: getPersonalSetting<boolean> cast site - DiscourseFloatingMenu: unused props parameter (no caller ever overrode default) - initializeObserversAndListeners: !== false dead pattern (Zod boolean default) - initializeObserversAndListeners: as IKeyCombo cast (schema is structurally compatible) AGENTS.md compliance for >2-arg functions: - mountLeftSidebar: object-destructured params, both call sites updated - installDiscourseFloatingMenu: kept at 2 positional via dead-props removal posthog: collapse doInitPostHog wrapper into initPostHog (caller-side gating). accessors: revert speculative readPathValue export (no consumer). LeftSidebarView/DiscourseFloatingMenu: eslint-disable react/no-deprecated on ReactDOM.render rewritten lines, matching existing codebase convention. * ENG-1616: Address review — rename snapshot vars, flag-gate bulkRead, move PostHog guards - Rename settingsSnapshot/callbackSnapshot/snap/navSnapshot → settings - bulkReadSettings now checks "Use new settings store" flag and falls back to legacy reads when off, matching individual getter behavior - Move encryption/offline guards into initPostHog (diagnostics check stays at call site to avoid race with async setSetting in enablePostHog) * Fix legacy bulk settings fallback
1 parent 86cf3f0 commit d8ff16f

19 files changed

Lines changed: 299 additions & 148 deletions

apps/roam/src/components/DiscourseFloatingMenu.tsx

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {
1313
import { FeedbackWidget } from "./BirdEatsBugs";
1414
import { render as renderSettings } from "~/components/settings/Settings";
1515
import posthog from "posthog-js";
16-
import { getPersonalSetting } from "./settings/utils/accessors";
16+
import { type SettingsSnapshot } from "./settings/utils/accessors";
1717
import { PERSONAL_KEYS } from "./settings/utils/settingKeys";
1818

1919
type DiscourseFloatingMenuProps = {
@@ -118,26 +118,23 @@ export const showDiscourseFloatingMenu = () => {
118118

119119
export const installDiscourseFloatingMenu = (
120120
onLoadArgs: OnloadArgs,
121-
props: DiscourseFloatingMenuProps = {
122-
position: "bottom-right",
123-
theme: "bp3-light",
124-
buttonTheme: "bp3-light",
125-
},
121+
snapshot: SettingsSnapshot,
126122
) => {
127123
let floatingMenuAnchor = document.getElementById(ANCHOR_ID);
128124
if (!floatingMenuAnchor) {
129125
floatingMenuAnchor = document.createElement("div");
130126
floatingMenuAnchor.id = ANCHOR_ID;
131127
document.getElementById("app")?.appendChild(floatingMenuAnchor);
132128
}
133-
if (getPersonalSetting<boolean>([PERSONAL_KEYS.hideFeedbackButton])) {
129+
if (snapshot.personalSettings[PERSONAL_KEYS.hideFeedbackButton]) {
134130
floatingMenuAnchor.classList.add("hidden");
135131
}
132+
// eslint-disable-next-line react/no-deprecated
136133
ReactDOM.render(
137134
<DiscourseFloatingMenu
138-
position={props.position}
139-
theme={props.theme}
140-
buttonTheme={props.buttonTheme}
135+
position="bottom-right"
136+
theme="bp3-light"
137+
buttonTheme="bp3-light"
141138
onloadArgs={onLoadArgs}
142139
/>,
143140
floatingMenuAnchor,
@@ -148,6 +145,7 @@ export const removeDiscourseFloatingMenu = () => {
148145
const anchor = document.getElementById(ANCHOR_ID);
149146
if (anchor) {
150147
try {
148+
// eslint-disable-next-line react/no-deprecated
151149
ReactDOM.unmountComponentAtNode(anchor);
152150
} catch (e) {
153151
// no-op: unmount best-effort

apps/roam/src/components/LeftSidebarView.tsx

Lines changed: 37 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import {
3939
getPersonalSettings,
4040
setGlobalSetting,
4141
setPersonalSetting,
42+
type SettingsSnapshot,
4243
} from "~/components/settings/utils/accessors";
4344
import {
4445
PERSONAL_KEYS,
@@ -336,14 +337,16 @@ const GlobalSection = ({ config }: { config: LeftSidebarConfig["global"] }) => {
336337

337338
// TODO(ENG-1471): Remove old-system merge when migration complete — just use accessor values directly.
338339
// See mergeGlobalSectionWithAccessor/mergePersonalSectionsWithAccessor for why the merge exists.
339-
const buildConfig = (): LeftSidebarConfig => {
340+
const buildConfig = (snapshot?: SettingsSnapshot): LeftSidebarConfig => {
340341
// Read VALUES from accessor (handles flag routing + mismatch detection)
341-
const globalValues = getGlobalSetting<LeftSidebarGlobalSettings>([
342-
GLOBAL_KEYS.leftSidebar,
343-
]);
344-
const personalValues = getPersonalSetting<
345-
ReturnType<typeof getPersonalSettings>[typeof PERSONAL_KEYS.leftSidebar]
346-
>([PERSONAL_KEYS.leftSidebar]);
342+
const globalValues = snapshot
343+
? snapshot.globalSettings[GLOBAL_KEYS.leftSidebar]
344+
: getGlobalSetting<LeftSidebarGlobalSettings>([GLOBAL_KEYS.leftSidebar]);
345+
const personalValues = snapshot
346+
? snapshot.personalSettings[PERSONAL_KEYS.leftSidebar]
347+
: getPersonalSetting<
348+
ReturnType<typeof getPersonalSettings>[typeof PERSONAL_KEYS.leftSidebar]
349+
>([PERSONAL_KEYS.leftSidebar]);
347350

348351
// Read UIDs from old system (needed for fold CRUD during dual-write)
349352
const oldConfig = getCurrentLeftSidebarConfig();
@@ -364,8 +367,8 @@ const buildConfig = (): LeftSidebarConfig => {
364367
};
365368
};
366369

367-
export const useConfig = () => {
368-
const [config, setConfig] = useState(() => buildConfig());
370+
export const useConfig = (initialSnapshot?: SettingsSnapshot) => {
371+
const [config, setConfig] = useState(() => buildConfig(initialSnapshot));
369372
useEffect(() => {
370373
const handleUpdate = () => {
371374
setConfig(buildConfig());
@@ -504,8 +507,14 @@ const FavoritesPopover = ({ onloadArgs }: { onloadArgs: OnloadArgs }) => {
504507
);
505508
};
506509

507-
const LeftSidebarView = ({ onloadArgs }: { onloadArgs: OnloadArgs }) => {
508-
const { config } = useConfig();
510+
const LeftSidebarView = ({
511+
onloadArgs,
512+
initialSnapshot,
513+
}: {
514+
onloadArgs: OnloadArgs;
515+
initialSnapshot?: SettingsSnapshot;
516+
}) => {
517+
const { config } = useConfig(initialSnapshot);
509518

510519
return (
511520
<>
@@ -610,10 +619,15 @@ const migrateFavorites = async () => {
610619
refreshConfigTree();
611620
};
612621

613-
export const mountLeftSidebar = async (
614-
wrapper: HTMLElement,
615-
onloadArgs: OnloadArgs,
616-
): Promise<void> => {
622+
export const mountLeftSidebar = async ({
623+
wrapper,
624+
onloadArgs,
625+
initialSnapshot,
626+
}: {
627+
wrapper: HTMLElement;
628+
onloadArgs: OnloadArgs;
629+
initialSnapshot?: SettingsSnapshot;
630+
}): Promise<void> => {
617631
if (!wrapper) return;
618632

619633
const id = "dg-left-sidebar-root";
@@ -630,7 +644,14 @@ export const mountLeftSidebar = async (
630644
} else {
631645
root.className = "starred-pages";
632646
}
633-
ReactDOM.render(<LeftSidebarView onloadArgs={onloadArgs} />, root);
647+
// eslint-disable-next-line react/no-deprecated
648+
ReactDOM.render(
649+
<LeftSidebarView
650+
onloadArgs={onloadArgs}
651+
initialSnapshot={initialSnapshot}
652+
/>,
653+
root,
654+
);
634655
};
635656

636657
export default LeftSidebarView;

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

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import type { OnloadArgs } from "roamjs-components/types";
55
import {
66
getPersonalSetting,
77
setPersonalSetting,
8+
type SettingsSnapshot,
89
} from "~/components/settings/utils/accessors";
910
import {
1011
PERSONAL_KEYS,
@@ -13,11 +14,13 @@ import {
1314

1415
// Legacy extensionAPI stored query-pages as string | string[] | Record<string, string>.
1516
// Coerce to string[] for backward compatibility with old stored formats.
16-
export const getQueryPages = (): string[] => {
17-
const value = getPersonalSetting<string[] | string | Record<string, string>>([
18-
PERSONAL_KEYS.query,
19-
QUERY_KEYS.queryPages,
20-
]);
17+
export const getQueryPages = (snapshot?: SettingsSnapshot): string[] => {
18+
const value: string[] | string | Record<string, string> | undefined = snapshot
19+
? snapshot.personalSettings[PERSONAL_KEYS.query][QUERY_KEYS.queryPages]
20+
: getPersonalSetting<string[] | string | Record<string, string>>([
21+
PERSONAL_KEYS.query,
22+
QUERY_KEYS.queryPages,
23+
]);
2124
return typeof value === "string"
2225
? [value]
2326
: Array.isArray(value)

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

Lines changed: 59 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -863,8 +863,10 @@ export const setGlobalSetting = (keys: string[], value: json): void => {
863863
});
864864
};
865865

866-
export const getAllRelations = (): DiscourseRelation[] => {
867-
const settings = getGlobalSettings();
866+
export const getAllRelations = (
867+
snapshot?: SettingsSnapshot,
868+
): DiscourseRelation[] => {
869+
const settings = snapshot ? snapshot.globalSettings : getGlobalSettings();
868870

869871
return Object.entries(settings.Relations).flatMap(([id, relation]) =>
870872
relation.ifConditions.map((ifCondition) => ({
@@ -909,6 +911,61 @@ export const getPersonalSetting = <T = unknown>(
909911
return blockPropsValue as T | undefined;
910912
};
911913

914+
export type SettingsSnapshot = {
915+
featureFlags: FeatureFlags;
916+
globalSettings: GlobalSettings;
917+
personalSettings: PersonalSettings;
918+
};
919+
920+
export const bulkReadSettings = (): SettingsSnapshot => {
921+
const pageResult = window.roamAlphaAPI.pull(
922+
"[{:block/children [:block/string :block/props]}]",
923+
[":node/title", DG_BLOCK_PROP_SETTINGS_PAGE_TITLE],
924+
) as Record<string, json> | null;
925+
926+
const children = (pageResult?.[":block/children"] ?? []) as Record<
927+
string,
928+
json
929+
>[];
930+
const personalKey = getPersonalSettingsKey();
931+
let featureFlagsProps: json = {};
932+
let globalProps: json = {};
933+
let personalProps: json = {};
934+
935+
for (const child of children) {
936+
const text = child[":block/string"];
937+
if (typeof text !== "string") continue;
938+
const rawBlockProps = child[":block/props"];
939+
const blockProps =
940+
rawBlockProps && typeof rawBlockProps === "object"
941+
? normalizeProps(rawBlockProps)
942+
: {};
943+
if (text === TOP_LEVEL_BLOCK_PROP_KEYS.featureFlags) {
944+
featureFlagsProps = blockProps;
945+
} else if (text === TOP_LEVEL_BLOCK_PROP_KEYS.global) {
946+
globalProps = blockProps;
947+
} else if (text === personalKey) {
948+
personalProps = blockProps;
949+
}
950+
}
951+
952+
const featureFlags = FeatureFlagsSchema.parse(featureFlagsProps || {});
953+
954+
if (!featureFlags["Use new settings store"]) {
955+
return {
956+
featureFlags,
957+
globalSettings: readAllLegacyGlobalSettings() as GlobalSettings,
958+
personalSettings: readAllLegacyPersonalSettings() as PersonalSettings,
959+
};
960+
}
961+
962+
return {
963+
featureFlags,
964+
globalSettings: GlobalSettingsSchema.parse(globalProps || {}),
965+
personalSettings: PersonalSettingsSchema.parse(personalProps || {}),
966+
};
967+
};
968+
912969
export const setPersonalSetting = (keys: string[], value: json): void => {
913970
if (keys.length === 0) {
914971
internalError({

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,9 @@ export type InitSchemaResult = {
336336
nodePageUids: Record<string, string>;
337337
};
338338

339+
// On-demand dual-read comparison. Not called automatically on init —
340+
// invoke from the console via window.dgDualReadLog() to inspect the legacy
341+
// settings tree vs. the block-prop store.
339342
const logDualReadComparison = (): void => {
340343
if (!isNewSettingsStoreEnabled()) return;
341344

@@ -415,11 +418,6 @@ export const initSchema = async (): Promise<InitSchemaResult> => {
415418
await migrateGraphLevel(blockUids);
416419
const nodePageUids = await initDiscourseNodePages();
417420
await migratePersonalSettings(blockUids);
418-
try {
419-
logDualReadComparison();
420-
} catch (e) {
421-
console.warn("[DG Dual-Read] Comparison failed:", e);
422-
}
423421
(window as unknown as Record<string, unknown>).dgDualReadLog =
424422
logDualReadComparison;
425423
return { blockUids, nodePageUids };

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

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import getBlockProps from "~/utils/getBlockProps";
22
import type { json } from "~/utils/getBlockProps";
33
import setBlockProps from "~/utils/setBlockProps";
4-
import getBlockUidByTextOnPage from "roamjs-components/queries/getBlockUidByTextOnPage";
54
import getPageUidByPageTitle from "roamjs-components/queries/getPageUidByPageTitle";
65
import { createBlock } from "roamjs-components/writes";
76
import { getSetting, setSetting } from "~/utils/extensionSettings";
@@ -29,11 +28,8 @@ const GRAPH_MIGRATION_MARKER = "Block props migrated";
2928
const PERSONAL_MIGRATION_MARKER = "dg-personal-settings-migrated";
3029
const MAX_ERROR_CONTEXT_LENGTH = 5000;
3130

32-
const hasGraphMigrationMarker = (): boolean =>
33-
!!getBlockUidByTextOnPage({
34-
text: GRAPH_MIGRATION_MARKER,
35-
title: DG_BLOCK_PROP_SETTINGS_PAGE_TITLE,
36-
});
31+
const hasGraphMigrationMarker = (blockMap: Record<string, string>): boolean =>
32+
!!blockMap[GRAPH_MIGRATION_MARKER];
3733

3834
const isPropsValid = (
3935
schema: z.ZodTypeAny,
@@ -182,7 +178,7 @@ export const migrateGraphLevel = async (
182178
return;
183179
}
184180

185-
if (hasGraphMigrationMarker()) {
181+
if (hasGraphMigrationMarker(blockUids)) {
186182
console.log(`${LOG_PREFIX} graph-level: skipped (already migrated)`);
187183
return;
188184
}

0 commit comments

Comments
 (0)