Skip to content

fix: update onSave to pass nested tree values and handle save errors #65

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

Merged
mrabbani merged 4 commits into from
Mar 6, 2026
Merged

fix: update onSave to pass nested tree values and handle save errors #65

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
23ed06d
fix: update onSave to pass nested tree values and handle save errors
mrabbani Mar 6, 2026
5061700
docs: update onSave signature in all documentation
mrabbani Mar 6, 2026
f32dad7
docs: add CLAUDE.md for AI-assisted development context
mrabbani Mar 6, 2026
5ea0769
fix: rename context onSave to save to disambiguate from consumer prop
mrabbani Mar 6, 2026
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
4 changes: 2 additions & 2 deletions DEVELOPER_GUIDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1128,11 +1128,11 @@ function SettingsPage() {
onChange={(scopeId, key, value) => {
setValues(prev => ({ ...prev, [key]: value }));
}}
onSave={async (scopeId, pageValues) => {
onSave={async (scopeId, treeValues) => {
await apiFetch({
path: `/my-plugin/v1/settings/${scopeId}`,
method: 'POST',
data: pageValues,
data: treeValues,
});
}}
renderSaveButton={({ dirty, onSave }) => (
Expand Down
4 changes: 2 additions & 2 deletions src/DeveloperGuide.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -912,8 +912,8 @@ function SettingsPage() {
hookPrefix="my_plugin"
applyFilters={applyFilters}
onChange={(scopeId, key, value) => setValues(prev => ({ ...prev, [key]: value }))}
onSave={async (scopeId, pageValues) => {
await apiFetch({ path: `/my-plugin/v1/settings/${scopeId}`, method: 'POST', data: pageValues });
onSave={async (scopeId, treeValues) => {
await apiFetch({ path: `/my-plugin/v1/settings/${scopeId}`, method: 'POST', data: treeValues });
}}
renderSaveButton={({ dirty, onSave }) => (
<Button onClick={onSave} disabled={!dirty}>{__('Save Changes', 'my-plugin')}</Button>
Expand Down
26 changes: 16 additions & 10 deletions src/components/settings/Settings.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,10 @@ function MySettingsPage() {
onChange={(scopeId, key, value) => {
setValues((prev) => ({ ...prev, [key]: value }));
}}
onSave={(scopeId, pageValues) => {
// POST pageValues to your REST API
console.log("Saving", scopeId, pageValues);
onSave={(scopeId, treeValues, flatValues) => {
// treeValues: nested object from dot-separated keys
// flatValues: original flat dot-keyed values
console.log("Saving", scopeId, treeValues, flatValues);
}}
renderSaveButton={({ dirty, onSave }) => (
<Button onClick={onSave} disabled={!dirty}>
Expand Down Expand Up @@ -528,7 +529,7 @@ Fired whenever a field value changes.
/>
```

### `onSave(scopeId, values)`
### `onSave(scopeId, treeValues, flatValues)`

Fired when the save button is clicked. Only receives values **scoped to the active page/subpage**.

Expand All @@ -547,19 +548,24 @@ Fired when the save button is clicked. Only receives values **scoped to the acti
<td>Active subpage ID, or page ID if no subpage</td>
</tr>
<tr>
<td><code>values</code></td>
<td><code>treeValues</code></td>
<td><code>{'Record<string, any>'}</code></td>
<td>Nested object built from dot-separated keys (e.g. <code>{'{"dokan":{"general":{"store_name":"..."}}}'}</code>)</td>
</tr>
<tr>
<td><code>flatValues</code></td>
<td><code>{'Record<string, any>'}</code></td>
<td>Key-value pairs for fields in this scope only</td>
<td>Original flat dot-keyed values (e.g. <code>{'{"dokan.general.store_name":"..."}'}</code>)</td>
</tr>
</tbody>
</table>

```tsx
<Settings
onSave={async (scopeId, pageValues) => {
onSave={async (scopeId, treeValues, flatValues) => {
await fetch(`/wp-json/my-plugin/v1/settings/${scopeId}`, {
method: "POST",
body: JSON.stringify(pageValues),
body: JSON.stringify(treeValues),
});
}}
/>
Expand Down Expand Up @@ -616,7 +622,7 @@ import { Settings, Button } from "@wedevs/plugin-ui";
<tr>
<td><code>onSave</code></td>
<td><code>{'() => void'}</code></td>
<td>Call this to trigger <code>onSave(scopeId, scopeValues)</code></td>
<td>Call this to trigger <code>onSave(scopeId, treeValues, flatValues)</code></td>
</tr>
</tbody>
</table>
Expand Down Expand Up @@ -810,7 +816,7 @@ const initialValues = extractValues(schema);
</tr>
<tr>
<td><code>onSave</code></td>
<td><code>{'(scopeId, values) => void'}</code></td>
<td><code>{'(scopeId, treeValues, flatValues) => void'}</code></td>
<td>—</td>
<td>Called on save; enables save button area</td>
</tr>
Expand Down
33 changes: 28 additions & 5 deletions src/components/settings/settings-context.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export interface SettingsContextValue {
/** Get only the values that belong to a specific page */
getPageValues: (pageId: string) => Record<string, any>;
/** Consumer-provided save handler (exposed so SettingsContent can call it) */
onSave?: (pageId: string, values: Record<string, any>) => void | Promise<void>;
onSave?: (pageId: string, treeValues: Record<string, any>, flatValues: Record<string, any>) => void | Promise<void>;
/** Consumer-provided render function for the save button */
renderSaveButton?: (props: SaveButtonRenderProps) => React.ReactNode;
}
Expand All @@ -88,7 +88,7 @@ export interface SettingsProviderProps {
schema: SettingsElement[];
values?: Record<string, any>;
onChange?: (scopeId: string, key: string, value: any) => void;
onSave?: (scopeId: string, values: Record<string, any>) => void | Promise<void>;
onSave?: (scopeId: string, treeValues: Record<string, any>, flatValues: Record<string, any>) => void | Promise<void>;

@coderabbitai coderabbitai Bot Mar 6, 2026

Copy link
Copy Markdown

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:

# First, read the settings-context.tsx file to see the context around line 91
head -100 src/components/settings/settings-context.tsx | tail -20

Repository: getdokan/plugin-ui

Length of output: 895

🏁 Script executed:

# Get more context - read the full file or at least the type/interface definition
wc -l src/components/settings/settings-context.tsx

Repository: getdokan/plugin-ui

Length of output: 110

🏁 Script executed:

# Search for onSave usages in the codebase
rg -n 'onSave' --type=ts --type=tsx -B 2 -A 2 | head -100

Repository: getdokan/plugin-ui

Length of output: 89

🏁 Script executed:

# Search for onSave usages without type filters
rg -n 'onSave' -B 2 -A 2

Repository: getdokan/plugin-ui

Length of output: 15495

🏁 Script executed:

# Also check how onSave is called/invoked in the file
rg -n 'onSave\s*\(' -B 1 -A 1

Repository: getdokan/plugin-ui

Length of output: 1625

🏁 Script executed:

# Verify the exact line in Settings.stories.tsx to understand the context better
sed -n '1080,1095p' src/components/settings/Settings.stories.tsx

Repository: getdokan/plugin-ui

Length of output: 911

🏁 Script executed:

# Check if there are any other invocations of onSave callback we might have missed
rg -n 'onSave\s*\(\s*[^)]*\s*\)' -B 1 -A 1

Repository: getdokan/plugin-ui

Length of output: 1625

Breaking change: onSave signature updated — some call sites still need updating.

The new signature (scopeId, treeValues, flatValues) is a breaking change. Several usages have not been updated:

  • src/components/settings/Settings.stories.tsx line 1084 still uses 2 parameters
  • src/DeveloperGuide.mdx and DEVELOPER_GUIDE.md examples still use 2 parameters

Update these call sites to accept the new third parameter flatValues or ignore it with a rest parameter if unused.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/components/settings/settings-context.tsx` at line 91, The onSave callback
signature in settings-context.tsx changed to onSave?: (scopeId: string,
treeValues: Record<string, any>, flatValues: Record<string, any>) => void |
Promise<void>, so update all call sites that still use two parameters (e.g., the
Settings.stories usage and the Developer Guide examples) to accept the third
flatValues argument or explicitly ignore it (for example change (scopeId,
treeValues) => ... to (scopeId, treeValues, flatValues) => ... or (scopeId,
treeValues, ..._rest) => ...), ensuring any handlers that call or implement
onSave (named onSave) match the new three-parameter signature.

renderSaveButton?: (props: SaveButtonRenderProps) => React.ReactNode;
loading?: boolean;
hookPrefix?: string;
Expand Down Expand Up @@ -261,10 +261,33 @@ export function SettingsProvider({
const handleOnSave = useCallback(
async (pageId: string, pageValues: Record<string, any>) => {
if (!onSave) return;
await Promise.resolve(onSave(pageId, pageValues));
resetPageDirty(pageId);
// Build nested tree from dot-separated keys
const treeValues: Record<string, any> = {};
for (const [dotKey, val] of Object.entries(pageValues)) {
const parts = dotKey.split('.');
let cursor: Record<string, any> = treeValues;
for (let i = 0; i < parts.length - 1; i++) {
if (!(parts[i] in cursor) || typeof cursor[parts[i]] !== 'object') {
cursor[parts[i]] = {};
}
cursor = cursor[parts[i]];
}
cursor[parts[parts.length - 1]] = val;
}
try {
await Promise.resolve(onSave(pageId, treeValues, pageValues));
resetPageDirty(pageId);
} catch (error: any) {
console.error('[Settings] onSave error caught:', error);
// If the error contains field-level errors (e.g. from a 400 response),
// merge them into the errors state so they display on the relevant fields.
// Error keys should match field dependency_key values.
if (error && typeof error === 'object' && error.errors && typeof error.errors === 'object') {
setErrors((prev) => ({ ...prev, ...error.errors }));
}
}
},
[onSave, resetPageDirty]
[onSave, resetPageDirty, setErrors]
);

// Update a field value
Expand Down
4 changes: 2 additions & 2 deletions src/components/settings/settings-types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,8 @@ export interface SettingsProps {
values?: Record<string, any>;
/** Called when a field value changes. Receives the scope ID (subpage/page), field key, and new value. */
onChange?: (scopeId: string, key: string, value: any) => void;
/** Called when the save button is clicked. Receives the scope ID and that scope's values only. */
onSave?: (scopeId: string, values: Record<string, any>) => void;
/** Called when the save button is clicked. Receives the scope ID, nested tree values, and flat dot-keyed values. */
onSave?: (scopeId: string, treeValues: Record<string, any>, flatValues: Record<string, any>) => void;
Comment on lines +139 to +140

@coderabbitai coderabbitai Bot Mar 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Return type mismatch: void vs void | Promise<void>.

The onSave return type here is void, but in settings-context.tsx (lines 72 and 91), the same callback is typed as void | Promise<void>. This inconsistency may cause TypeScript errors for consumers using async onSave handlers.

Since handleOnSave in the context uses await Promise.resolve(onSave(...)), the Promise return type should be supported:

     /** Called when the save button is clicked. Receives the scope ID, nested tree values, and flat dot-keyed values. */
-    onSave?: (scopeId: string, treeValues: Record<string, any>, flatValues: Record<string, any>) => void;
+    onSave?: (scopeId: string, treeValues: Record<string, any>, flatValues: Record<string, any>) => void | Promise<void>;
📝 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
/** Called when the save button is clicked. Receives the scope ID, nested tree values, and flat dot-keyed values. */
onSave?: (scopeId: string, treeValues: Record<string, any>, flatValues: Record<string, any>) => void;
/** Called when the save button is clicked. Receives the scope ID, nested tree values, and flat dot-keyed values. */
onSave?: (scopeId: string, treeValues: Record<string, any>, flatValues: Record<string, any>) => void | Promise<void>;
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/components/settings/settings-types.ts` around lines 139 - 140, The onSave
callback in the settings types is currently declared to return void but must
allow async handlers; update the onSave signature in settings-types (the
onSave?: (scopeId: string, treeValues: Record<string, any>, flatValues:
Record<string, any>) => void) to return void | Promise<void> so it matches how
handleOnSave in settings-context.tsx awaits Promise.resolve(onSave(...)) and
avoids TypeScript mismatches.

/**
* Custom render function for the save button area.
* Use this to provide your own translated save button.
Expand Down
Loading