Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 8 additions & 10 deletions apps/roam/src/components/DiscourseFloatingMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
import { FeedbackWidget } from "./BirdEatsBugs";
import { render as renderSettings } from "~/components/settings/Settings";
import posthog from "posthog-js";
import { getPersonalSetting } from "./settings/utils/accessors";
import { type SettingsSnapshot } from "./settings/utils/accessors";
import { PERSONAL_KEYS } from "./settings/utils/settingKeys";

type DiscourseFloatingMenuProps = {
Expand Down Expand Up @@ -118,26 +118,23 @@ export const showDiscourseFloatingMenu = () => {

export const installDiscourseFloatingMenu = (
onLoadArgs: OnloadArgs,
props: DiscourseFloatingMenuProps = {
position: "bottom-right",
theme: "bp3-light",
buttonTheme: "bp3-light",
},
snapshot: SettingsSnapshot,
) => {
let floatingMenuAnchor = document.getElementById(ANCHOR_ID);
if (!floatingMenuAnchor) {
floatingMenuAnchor = document.createElement("div");
floatingMenuAnchor.id = ANCHOR_ID;
document.getElementById("app")?.appendChild(floatingMenuAnchor);
}
if (getPersonalSetting<boolean>([PERSONAL_KEYS.hideFeedbackButton])) {
if (snapshot.personalSettings[PERSONAL_KEYS.hideFeedbackButton]) {
floatingMenuAnchor.classList.add("hidden");
}
// eslint-disable-next-line react/no-deprecated
ReactDOM.render(
<DiscourseFloatingMenu
position={props.position}
theme={props.theme}
buttonTheme={props.buttonTheme}
position="bottom-right"
theme="bp3-light"
buttonTheme="bp3-light"
onloadArgs={onLoadArgs}
/>,
floatingMenuAnchor,
Expand All @@ -148,6 +145,7 @@ export const removeDiscourseFloatingMenu = () => {
const anchor = document.getElementById(ANCHOR_ID);
if (anchor) {
try {
// eslint-disable-next-line react/no-deprecated
ReactDOM.unmountComponentAtNode(anchor);
} catch (e) {
// no-op: unmount best-effort
Expand Down
53 changes: 37 additions & 16 deletions apps/roam/src/components/LeftSidebarView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import {
getPersonalSettings,
setGlobalSetting,
setPersonalSetting,
type SettingsSnapshot,
} from "~/components/settings/utils/accessors";
import {
PERSONAL_KEYS,
Expand Down Expand Up @@ -336,14 +337,16 @@ const GlobalSection = ({ config }: { config: LeftSidebarConfig["global"] }) => {

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

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

export const useConfig = () => {
const [config, setConfig] = useState(() => buildConfig());
export const useConfig = (initialSnapshot?: SettingsSnapshot) => {
const [config, setConfig] = useState(() => buildConfig(initialSnapshot));
useEffect(() => {
const handleUpdate = () => {
setConfig(buildConfig());
Expand Down Expand Up @@ -504,8 +507,14 @@ const FavoritesPopover = ({ onloadArgs }: { onloadArgs: OnloadArgs }) => {
);
};

const LeftSidebarView = ({ onloadArgs }: { onloadArgs: OnloadArgs }) => {
const { config } = useConfig();
const LeftSidebarView = ({
onloadArgs,
initialSnapshot,
}: {
onloadArgs: OnloadArgs;
initialSnapshot?: SettingsSnapshot;
}) => {
const { config } = useConfig(initialSnapshot);

return (
<>
Expand Down Expand Up @@ -610,10 +619,15 @@ const migrateFavorites = async () => {
refreshConfigTree();
};

export const mountLeftSidebar = async (
wrapper: HTMLElement,
onloadArgs: OnloadArgs,
): Promise<void> => {
export const mountLeftSidebar = async ({
wrapper,
onloadArgs,
initialSnapshot,
}: {
wrapper: HTMLElement;
onloadArgs: OnloadArgs;
initialSnapshot?: SettingsSnapshot;
}): Promise<void> => {
if (!wrapper) return;

const id = "dg-left-sidebar-root";
Expand All @@ -630,7 +644,14 @@ export const mountLeftSidebar = async (
} else {
root.className = "starred-pages";
}
ReactDOM.render(<LeftSidebarView onloadArgs={onloadArgs} />, root);
// eslint-disable-next-line react/no-deprecated
ReactDOM.render(
<LeftSidebarView
onloadArgs={onloadArgs}
initialSnapshot={initialSnapshot}
/>,
root,
);
};

export default LeftSidebarView;
13 changes: 8 additions & 5 deletions apps/roam/src/components/settings/QueryPagesPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type { OnloadArgs } from "roamjs-components/types";
import {
getPersonalSetting,
setPersonalSetting,
type SettingsSnapshot,
} from "~/components/settings/utils/accessors";
import {
PERSONAL_KEYS,
Expand All @@ -13,11 +14,13 @@ import {

// Legacy extensionAPI stored query-pages as string | string[] | Record<string, string>.
// Coerce to string[] for backward compatibility with old stored formats.
export const getQueryPages = (): string[] => {
const value = getPersonalSetting<string[] | string | Record<string, string>>([
PERSONAL_KEYS.query,
QUERY_KEYS.queryPages,
]);
export const getQueryPages = (snapshot?: SettingsSnapshot): string[] => {
const value: string[] | string | Record<string, string> | undefined = snapshot
? snapshot.personalSettings[PERSONAL_KEYS.query][QUERY_KEYS.queryPages]
: getPersonalSetting<string[] | string | Record<string, string>>([
PERSONAL_KEYS.query,
QUERY_KEYS.queryPages,
]);
return typeof value === "string"
? [value]
: Array.isArray(value)
Expand Down
61 changes: 59 additions & 2 deletions apps/roam/src/components/settings/utils/accessors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -863,8 +863,10 @@ export const setGlobalSetting = (keys: string[], value: json): void => {
});
};

export const getAllRelations = (): DiscourseRelation[] => {
const settings = getGlobalSettings();
export const getAllRelations = (
snapshot?: SettingsSnapshot,
): DiscourseRelation[] => {
const settings = snapshot ? snapshot.globalSettings : getGlobalSettings();

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

export type SettingsSnapshot = {
featureFlags: FeatureFlags;
globalSettings: GlobalSettings;
personalSettings: PersonalSettings;
};

export const bulkReadSettings = (): SettingsSnapshot => {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe getPersonalSetting was checking if isNewSettingsStoreEnabled, but it doesn't look like bulkReadSettings is doing this anymore.

Specifically example would be: const disallowDiagnostics = settingsSnapshot.personalSettings[PERSONAL_KEYS.disableProductDiagnostics]; If the new store settings aren't enabled, would the migration have happened yet?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added the check, I missed it.

const pageResult = window.roamAlphaAPI.pull(
"[{:block/children [:block/string :block/props]}]",
[":node/title", DG_BLOCK_PROP_SETTINGS_PAGE_TITLE],
) as Record<string, json> | null;

const children = (pageResult?.[":block/children"] ?? []) as Record<
string,
json
>[];
const personalKey = getPersonalSettingsKey();
let featureFlagsProps: json = {};
let globalProps: json = {};
let personalProps: json = {};

for (const child of children) {
const text = child[":block/string"];
if (typeof text !== "string") continue;
const rawBlockProps = child[":block/props"];
const blockProps =
rawBlockProps && typeof rawBlockProps === "object"
? normalizeProps(rawBlockProps)
: {};
if (text === TOP_LEVEL_BLOCK_PROP_KEYS.featureFlags) {
featureFlagsProps = blockProps;
} else if (text === TOP_LEVEL_BLOCK_PROP_KEYS.global) {
globalProps = blockProps;
} else if (text === personalKey) {
personalProps = blockProps;
}
}

const featureFlags = FeatureFlagsSchema.parse(featureFlagsProps || {});

if (!featureFlags["Use new settings store"]) {
return {
featureFlags,
globalSettings: readAllLegacyGlobalSettings() as GlobalSettings,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

personalSettings: readAllLegacyPersonalSettings() as PersonalSettings,
};
Comment thread
sid597 marked this conversation as resolved.
}

return {
featureFlags,
globalSettings: GlobalSettingsSchema.parse(globalProps || {}),
personalSettings: PersonalSettingsSchema.parse(personalProps || {}),
};
};

export const setPersonalSetting = (keys: string[], value: json): void => {
if (keys.length === 0) {
internalError({
Expand Down
8 changes: 3 additions & 5 deletions apps/roam/src/components/settings/utils/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,9 @@ export type InitSchemaResult = {
nodePageUids: Record<string, string>;
};

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

Expand Down Expand Up @@ -415,11 +418,6 @@ export const initSchema = async (): Promise<InitSchemaResult> => {
await migrateGraphLevel(blockUids);
const nodePageUids = await initDiscourseNodePages();
await migratePersonalSettings(blockUids);
try {
logDualReadComparison();
} catch (e) {
console.warn("[DG Dual-Read] Comparison failed:", e);
}
(window as unknown as Record<string, unknown>).dgDualReadLog =
logDualReadComparison;
return { blockUids, nodePageUids };
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import getBlockProps from "~/utils/getBlockProps";
import type { json } from "~/utils/getBlockProps";
import setBlockProps from "~/utils/setBlockProps";
import getBlockUidByTextOnPage from "roamjs-components/queries/getBlockUidByTextOnPage";
import getPageUidByPageTitle from "roamjs-components/queries/getPageUidByPageTitle";
import { createBlock } from "roamjs-components/writes";
import { getSetting, setSetting } from "~/utils/extensionSettings";
Expand Down Expand Up @@ -29,11 +28,8 @@ const GRAPH_MIGRATION_MARKER = "Block props migrated";
const PERSONAL_MIGRATION_MARKER = "dg-personal-settings-migrated";
const MAX_ERROR_CONTEXT_LENGTH = 5000;

const hasGraphMigrationMarker = (): boolean =>
!!getBlockUidByTextOnPage({
text: GRAPH_MIGRATION_MARKER,
title: DG_BLOCK_PROP_SETTINGS_PAGE_TITLE,
});
const hasGraphMigrationMarker = (blockMap: Record<string, string>): boolean =>
!!blockMap[GRAPH_MIGRATION_MARKER];

const isPropsValid = (
schema: z.ZodTypeAny,
Expand Down Expand Up @@ -182,7 +178,7 @@ export const migrateGraphLevel = async (
return;
}

if (hasGraphMigrationMarker()) {
if (hasGraphMigrationMarker(blockUids)) {
console.log(`${LOG_PREFIX} graph-level: skipped (already migrated)`);
return;
}
Expand Down
Loading
Loading