Skip to content
Open
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
11 changes: 11 additions & 0 deletions .changeset/fix-create-modal-cached-data.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
"@refinedev/refine-core": patch
"@refinedev/refine-react-hook-form": patch
---

fix(core, react-hook-form): prevent cached show-page data from overwriting create modal defaultValues

When opening a create modal on a show page for the same resource, the form's `defaultValues` were overwritten by cached data from the show page's `useOne` query. This happened because `useForm` passed the URL-derived `id` to `useOne` even for create actions, causing a query key collision with the cached entry.

- **core:** Don't pass `id` to `useOne` for create actions, preventing the cache key collision at the source.
- **react-hook-form:** Guard the `useModalForm` visibility reset effect against create actions.
7 changes: 7 additions & 0 deletions .changeset/safe-route-id-decoding.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@refinedev/core": patch
"@refinedev/react-router": patch
"@refinedev/remix-router": patch
---

Safely handle malformed URI-encoded route params when parsing route ids and query targets.
19 changes: 14 additions & 5 deletions packages/antd/src/components/themedLayout/sider/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
Grid,
Drawer,
Button,
Tooltip,
theme,
ConfigProvider,
} from "antd";
Expand Down Expand Up @@ -82,7 +83,11 @@ export const ThemedSider: React.FC<RefineThemedLayoutSiderProps> = ({
<Menu.SubMenu
key={item.key}
icon={icon ?? <UnorderedListOutlined />}
title={label}
title={
<Tooltip title={siderCollapsed ? label : undefined}>
{label}
</Tooltip>
}
>
{renderTreeView(children, selectedKey)}
</Menu.SubMenu>
Expand All @@ -109,9 +114,11 @@ export const ThemedSider: React.FC<RefineThemedLayoutSiderProps> = ({
icon={icon ?? (isRoute && <UnorderedListOutlined />)}
style={linkStyle}
>
<Link to={route ?? ""} style={linkStyle}>
{label}
</Link>
<Tooltip title={siderCollapsed ? label : undefined}>
<Link to={route ?? ""} style={linkStyle}>
{label}
</Link>
</Tooltip>
{!siderCollapsed && isSelected && (
<div className="ant-menu-tree-arrow" />
)}
Expand Down Expand Up @@ -145,7 +152,9 @@ export const ThemedSider: React.FC<RefineThemedLayoutSiderProps> = ({
onClick={() => handleLogout()}
icon={<LogoutOutlined />}
>
{translate("buttons.logout", "Logout")}
<Tooltip title={siderCollapsed ? translate("buttons.logout", "Logout") : undefined}>
{translate("buttons.logout", "Logout")}
</Tooltip>
</Menu.Item>
);

Expand Down
10 changes: 9 additions & 1 deletion packages/core/src/definitions/helpers/handleUseParams/index.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,16 @@
const safeDecodeURIComponent = (value: string) => {
try {
return decodeURIComponent(value);
} catch {
return value;
}
};

export const handleUseParams = (params: any = {}): any => {
if (params?.id) {
return {
...params,
id: decodeURIComponent(params.id),
id: safeDecodeURIComponent(params.id),
};
}
return params;
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/hooks/form/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ export const useForm = <

const queryResult = useOne<TQueryFnData, TError, TData>({
resource: identifier,
id,
id: isCreate ? undefined : id,
queryOptions: {
// Only enable the query if it's not a create action and the `id` is defined
...props.queryOptions,
Expand Down
4 changes: 2 additions & 2 deletions packages/react-hook-form/src/useModalForm/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ export const useModalForm = <

// compensate for setting of initial form values in useForm since it doesnt track modal visibility
React.useEffect(() => {
if (!visible || !query?.data?.data) return;
if (!visible || !query?.data?.data || action === "create") return;

const formData = query.data.data;
if (!formData) return;
Expand All @@ -197,7 +197,7 @@ export const useModalForm = <
keepDirtyValues: true,
}),
});
}, [visible, query?.data?.data, autoResetFormWhenClose]);
}, [visible, query?.data?.data, autoResetFormWhenClose, action]);

React.useEffect(() => {
if (initiallySynced === false && syncWithLocationKey) {
Expand Down
7 changes: 5 additions & 2 deletions packages/react-hook-form/test/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
type IResourceItem,
type I18nProvider,
type IRefineOptions,
type RouterProvider,
} from "@refinedev/core";

import { MockJSONServer, mockRouterProvider } from "./dataMocks";
Expand All @@ -15,6 +16,7 @@ interface ITestWrapperProps {
dataProvider?: DataProvider;
resources?: IResourceItem[];
routerInitialEntries?: string[];
routerProvider?: RouterProvider;
i18nProvider?: I18nProvider;
options?: IRefineOptions;
}
Expand All @@ -25,6 +27,7 @@ export const TestWrapper: (
dataProvider,
resources,
routerInitialEntries,
routerProvider,
i18nProvider,
options,
}) => {
Expand All @@ -34,12 +37,12 @@ export const TestWrapper: (
<Refine
i18nProvider={i18nProvider}
dataProvider={dataProvider ?? MockJSONServer}
routerProvider={mockRouterProvider()}
routerProvider={routerProvider ?? mockRouterProvider()}
resources={resources ?? [{ name: "posts" }]}
options={{
...options,
reactQuery: {
clientConfig: {
clientConfig: options?.reactQuery?.clientConfig ?? {
defaultOptions: {
queries: {
retry: false,
Expand Down
5 changes: 3 additions & 2 deletions packages/react-router/src/bindings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
useParams,
} from "react-router";
import { convertToNumberIfPossible } from "./convert-to-number-if-possible";
import { safeDecodeURIComponent } from "./safe-decode-uri-component";

export const stringifyConfig = {
addQueryPrefix: true,
Expand Down Expand Up @@ -124,7 +125,7 @@ export const routerProvider: RouterProvider = {
const response: ParseResponse = {
...(resource && { resource }),
...(action && { action }),
...(params?.id && { id: decodeURIComponent(params.id) }),
...(params?.id && { id: safeDecodeURIComponent(params.id) }),
// ...(params?.action && { action: params.action }), // lets see if there is a need for this
pathname,
params: {
Expand All @@ -136,7 +137,7 @@ export const routerProvider: RouterProvider = {
combinedParams.pageSize as string,
) as number | undefined,
to: combinedParams.to
? decodeURIComponent(combinedParams.to as string)
? safeDecodeURIComponent(combinedParams.to as string)
: undefined,
},
};
Expand Down
7 changes: 7 additions & 0 deletions packages/react-router/src/safe-decode-uri-component.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export const safeDecodeURIComponent = (value: string) => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: The safeDecodeURIComponent utility is duplicated in three separate packages (core inline, react-router, remix-router) with identical logic. Since both router packages already depend on @refinedev/core as a peer dependency, this utility could be exported from core once and imported by the router packages, eliminating the duplication and reducing the risk of behavioral drift.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/react-router/src/safe-decode-uri-component.ts, line 1:

<comment>The `safeDecodeURIComponent` utility is duplicated in three separate packages (core inline, react-router, remix-router) with identical logic. Since both router packages already depend on `@refinedev/core` as a peer dependency, this utility could be exported from core once and imported by the router packages, eliminating the duplication and reducing the risk of behavioral drift.</comment>

<file context>
@@ -0,0 +1,7 @@
+export const safeDecodeURIComponent = (value: string) => {
+  try {
+    return decodeURIComponent(value);
</file context>

try {
return decodeURIComponent(value);
} catch {
return value;
}
};
7 changes: 4 additions & 3 deletions packages/remix-router/src/bindings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import qs from "qs";
import React, { type ComponentProps, useCallback, useContext } from "react";
import { paramsFromCurrentPath } from "./params-from-current-path";
import { convertToNumberIfPossible } from "./convert-to-number-if-possible";
import { safeDecodeURIComponent } from "./safe-decode-uri-component";

export const stringifyConfig = {
addQueryPrefix: true,
Expand Down Expand Up @@ -116,8 +117,8 @@ export const routerProvider: RouterProvider = {
const response: ParseResponse = {
...(resource && { resource }),
...(action && { action }),
...(inferredId && { id: decodeURIComponent(inferredId) }),
...(params?.id && { id: decodeURIComponent(params.id) }),
...(inferredId && { id: safeDecodeURIComponent(inferredId) }),
...(params?.id && { id: safeDecodeURIComponent(params.id) }),
// ...(params?.action && { action: params.action }), // lets see if there is a need for this
pathname,
params: {
Expand All @@ -129,7 +130,7 @@ export const routerProvider: RouterProvider = {
combinedParams.pageSize as string,
) as number | undefined,
to: combinedParams.to
? decodeURIComponent(combinedParams.to as string)
? safeDecodeURIComponent(combinedParams.to as string)
: undefined,
},
};
Expand Down
7 changes: 7 additions & 0 deletions packages/remix-router/src/safe-decode-uri-component.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export const safeDecodeURIComponent = (value: string) => {
try {
return decodeURIComponent(value);
} catch {
return value;
}
};
Loading