Skip to content

ENG-1191: add new setting component for feature flag #633

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sid597 wants to merge 2 commits into
base: eng-1188-create-a-pull-watch-handler-on-the-settings-page-per-feature
Choose a base branch
from
+78 −0
Open

ENG-1191: add new setting component for feature flag #633

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
25706dc
add new setting component for feature flag
sid597 Dec 22, 2025
d583c3d
validate on write
sid597 Dec 22, 2025
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
37 changes: 37 additions & 0 deletions apps/roam/src/components/settings/BlockPropFeatureFlagPanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { getFeatureFlag, setFeatureFlag } from "~/utils/manageFeatureFlag";
import { type FeatureFlags } from "~/utils/zodSchemaForSettings";
import { Checkbox } from "@blueprintjs/core";
import Description from "roamjs-components/components/Description";
import idToTitle from "roamjs-components/util/idToTitle";
import React, { useState } from "react";

export const BlockPropFeatureFlagPanel = ({
title,
description,
featureKey,
}: {
title: string;
description: string;
featureKey: keyof FeatureFlags;
}) => {
Comment on lines +8 to +16
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 22, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Add explicit return type annotation.

Per coding guidelines, functions should have explicit return types. Add : React.JSX.Element or : JSX.Element to the component signature.

🔎 Proposed fix 
 export const BlockPropFeatureFlagPanel = ({
   title,
   description,
   featureKey,
 }: {
   title: string;
   description: string;
   featureKey: keyof FeatureFlags;
-}) => {
+}): React.JSX.Element => {

As per coding guidelines, "Use explicit return types for functions."

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const BlockPropFeatureFlagPanel = ({
title,
description,
featureKey,
}: {
title: string;
description: string;
featureKey: keyof FeatureFlags;
}) => {
export const BlockPropFeatureFlagPanel = ({
title,
description,
featureKey,
}: {
title: string;
description: string;
featureKey: keyof FeatureFlags;
}): React.JSX.Element => {
🤖 Prompt for AI Agents 
In apps/roam/src/components/settings/BlockPropFeatureFlagPanel.tsx around lines
8-16, the functional component is missing an explicit return type; update the
component signature to include a React return type (e.g., add `:
React.JSX.Element` or `: JSX.Element` after the parameter list) so the function
has an explicit return type per coding guidelines; ensure necessary React types
are available from imports (add `import React from "react"` or import JSX types
if your TSX setup requires it) and then re-run typechecks.

const [value, setValue] = useState(() => getFeatureFlag(featureKey));

const handleChange = (e: React.ChangeEvent<HTMLInputElement>) => {
const { checked } = e.target;
setFeatureFlag(featureKey, checked);
setValue(checked);
};

return (
<Checkbox
checked={value}
onChange={handleChange}
labelElement={
<>
{idToTitle(title)}
<Description description={description} />
</>
}
/>
);
};
27 changes: 27 additions & 0 deletions apps/roam/src/utils/manageFeatureFlag.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { FeatureFlagsSchema, type FeatureFlags } from "./zodSchemaForSettings";
import { TOP_LEVEL_BLOCK_PROP_KEYS } from "~/data/blockPropsSettingsConfig";
import {
setBlockPropBasedSettings,
getBlockPropBasedSettings,
} from "~/utils/settingsUsingBlockProps";

export const getFeatureFlag = (key: keyof FeatureFlags): boolean => {
const featureFlagKey = TOP_LEVEL_BLOCK_PROP_KEYS.featureFlags;

const { blockProps } = getBlockPropBasedSettings({
keys: [featureFlagKey],
});

const flags = FeatureFlagsSchema.parse(blockProps || {});

return flags[key];
};
Comment on lines +9 to +19
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 22, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

getFeatureFlag will fail with the current empty schema.

Since FeatureFlagsSchema is an empty object, flags[key] at Line 17 attempts to access a property that doesn't exist on the validated object type. TypeScript treats keyof FeatureFlags as never, and runtime access will return undefined (not boolean).

Once feature flags are added to the schema, consider adding a fallback default value to handle cases where a flag isn't yet defined:

🔎 Suggested improvement for robustness 
 export const getFeatureFlag = (key: keyof FeatureFlags): boolean => {
   const featureFlagKey = TOP_LEVEL_BLOCK_PROP_KEYS.featureFlags;

   const { blockProps } = getBlockPropBasedSettings({
     keys: [featureFlagKey],
   });

   const flags = FeatureFlagsSchema.parse(blockProps || {});

-  return flags[key];
+  return flags[key] ?? false; // Default to false if key is missing
 };

This ensures the function always returns a boolean even if a flag isn't explicitly set in storage.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const getFeatureFlag = (key: keyof FeatureFlags): boolean => {
const featureFlagKey = TOP_LEVEL_BLOCK_PROP_KEYS.featureFlags;
const { blockProps } = getBlockPropBasedSettings({
keys: [featureFlagKey],
});
const flags = FeatureFlagsSchema.parse(blockProps || {});
return flags[key];
};
export const getFeatureFlag = (key: keyof FeatureFlags): boolean => {
const featureFlagKey = TOP_LEVEL_BLOCK_PROP_KEYS.featureFlags;
const { blockProps } = getBlockPropBasedSettings({
keys: [featureFlagKey],
});
const flags = FeatureFlagsSchema.parse(blockProps || {});
return flags[key] ?? false; // Default to false if key is missing
};
🤖 Prompt for AI Agents 
In apps/roam/src/utils/manageFeatureFlag.ts around lines 8–18, getFeatureFlag
currently reads flags[key] from an empty/partial schema which can be undefined
and TypeScript treats keyof FeatureFlags as never; change the function to always
return a boolean by providing a safe fallback and correct typing: ensure
FeatureFlagsSchema.parse returns a partial/optional shape or cast the parsed
result to Partial<FeatureFlags>, then return a boolean using a fallback (e.g.,
flags[key] ?? false or Boolean(flags[key])) so the function never returns
undefined.


export const setFeatureFlag = (
key: keyof FeatureFlags,
value: boolean,
): void => {
const featureFlagKey = TOP_LEVEL_BLOCK_PROP_KEYS.featureFlags;

void setBlockPropBasedSettings({ keys: [featureFlagKey, key], value });
};
8 changes: 8 additions & 0 deletions apps/roam/src/utils/zodSchemaForSettings.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { z } from "zod";

/* eslint-disable @typescript-eslint/naming-convention */
export const FeatureFlagsSchema = z.object({
});
Comment on lines +4 to +5
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 22, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

find apps/roam -type f -name "*zodSchemaForSettings*" 2>/dev/null

Repository: DiscourseGraphs/discourse-graph

Length of output: 118

🏁 Script executed:

cat -n apps/roam/src/utils/zodSchemaForSettings.ts

Repository: DiscourseGraphs/discourse-graph

Length of output: 385

🏁 Script executed:

rg -n "FeatureFlagsSchema|getFeatureFlag|setFeatureFlag|BlockPropFeatureFlagPanel" apps/roam/src --type ts --type tsx

Repository: DiscourseGraphs/discourse-graph

Length of output: 102

🏁 Script executed:

rg -n "FeatureFlag" apps/roam/src -A 3 -B 1 --type ts --type tsx

Repository: DiscourseGraphs/discourse-graph

Length of output: 102

🏁 Script executed:

rg -n "zodSchemaForSettings" apps/roam/src --type ts --type tsx

Repository: DiscourseGraphs/discourse-graph

Length of output: 102

🏁 Script executed:

rg -n "import.*FeatureFlagsSchema|from.*zodSchemaForSettings" apps/roam/src --type ts --type tsx

Repository: DiscourseGraphs/discourse-graph

Length of output: 102

🏁 Script executed:

find apps/roam/src -type f \( -name "*.ts" -o -name "*.tsx" \) -exec grep -l "feature\|flag" {} \; 2>/dev/null

Repository: DiscourseGraphs/discourse-graph

Length of output: 568

🏁 Script executed:

cd apps/roam/src/utils && git log --oneline -n 20 zodSchemaForSettings.ts 2>/dev/null || echo "No git history available"

Repository: DiscourseGraphs/discourse-graph

Length of output: 125

🏁 Script executed:

head -20 apps/roam/src/utils/zodSchemaForSettings.ts

Repository: DiscourseGraphs/discourse-graph

Length of output: 329

🏁 Script executed:

rg -n "TODO|FIXME|feature.flag|TODO.*feature" apps/roam/src --type ts --type tsx -i | head -20

Repository: DiscourseGraphs/discourse-graph

Length of output: 102

🏁 Script executed:

cat -n apps/roam/src/components/settings/BlockPropFeatureFlagPanel.tsx

Repository: DiscourseGraphs/discourse-graph

Length of output: 1329

🏁 Script executed:

cat -n apps/roam/src/utils/isFlagEnabled.ts

Repository: DiscourseGraphs/discourse-graph

Length of output: 874

🏁 Script executed:

cat -n apps/roam/src/utils/manageFeatureFlag.ts

Repository: DiscourseGraphs/discourse-graph

Length of output: 1073

🏁 Script executed:

cat -n apps/roam/src/utils/pullWatchBlockPropsBasedSettings.ts | head -50

Repository: DiscourseGraphs/discourse-graph

Length of output: 720

🏁 Script executed:

cat -n apps/roam/src/data/blockPropsSettingsConfig.ts

Repository: DiscourseGraphs/discourse-graph

Length of output: 259

Add at least one feature flag definition to make the type system functional.

FeatureFlagsSchema is empty, making keyof FeatureFlags resolve to never. The component BlockPropFeatureFlagPanel and functions getFeatureFlag/setFeatureFlag cannot accept any flag name, preventing the feature flag system from being used.

Add at least one flag definition:

 export const FeatureFlagsSchema = z.object({
+  exampleFeature: z.boolean().default(false),
 });

Alternatively, document in a comment that this PR establishes foundational infrastructure with flag definitions to be added in follow-up PRs.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const FeatureFlagsSchema = z.object({
});
export const FeatureFlagsSchema = z.object({
exampleFeature: z.boolean().default(false),
});
🤖 Prompt for AI Agents 
In apps/roam/src/utils/zodSchemaForSettings.ts around lines 4-5,
FeatureFlagsSchema is currently empty which causes keyof FeatureFlags to be
never and breaks consumers; either add at least one concrete flag definition
(e.g., a boolean flag name with z.boolean()) so the type system yields usable
keys, or add a clear file comment stating this PR only adds infrastructure and
that actual flag entries will be added in a follow-up PR — update exports/types
accordingly so BlockPropFeatureFlagPanel, getFeatureFlag and setFeatureFlag
accept real flag names.

/* eslint-disable @typescript-eslint/naming-convention */
Comment on lines +3 to +6
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 22, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix the ESLint comment pairing.

Line 6 duplicates the eslint-disable comment instead of re-enabling the rule. This leaves the naming-convention rule disabled for the remainder of the file.

🔎 Proposed fix 
 /* eslint-disable @typescript-eslint/naming-convention */
 export const FeatureFlagsSchema = z.object({
 });
-/* eslint-disable @typescript-eslint/naming-convention */
+/* eslint-enable @typescript-eslint/naming-convention */
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/* eslint-disable @typescript-eslint/naming-convention */
export const FeatureFlagsSchema = z.object({
});
/* eslint-disable @typescript-eslint/naming-convention */
/* eslint-disable @typescript-eslint/naming-convention */
export const FeatureFlagsSchema = z.object({
});
/* eslint-enable @typescript-eslint/naming-convention */
🤖 Prompt for AI Agents 
In apps/roam/src/utils/zodSchemaForSettings.ts around lines 3 to 6, the ESLint
comment at line 6 mistakenly duplicates `/* eslint-disable
@typescript-eslint/naming-convention */` instead of re-enabling the rule; change
the second comment to `/* eslint-enable @typescript-eslint/naming-convention */`
(or remove the disable and scope the disable/enable pair properly around the
specific code) so the naming-convention rule is only disabled for the intended
block.


export type FeatureFlags = z.infer<typeof FeatureFlagsSchema>;