Skip to content
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.

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

Fix bug with search params removal #9969

Merged
merged 3 commits into from
Jan 24, 2023
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
6 changes: 6 additions & 0 deletions .changeset/afraid-ducks-know.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"react-router-dom": patch
"react-router-native": patch
---

Fix bug with search params removal
36 changes: 36 additions & 0 deletions packages/react-router-dom/__tests__/search-params-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -125,4 +125,40 @@ describe("useSearchParams", () => {
);
expect(node.innerHTML).toMatch(/The new query is "Ryan Florence"/);
});

it("allows removal of search params when a default is provided", () => {
function SearchPage() {
let [searchParams, setSearchParams] = useSearchParams({
value: "initial",
});

return (
<div>
<p>The current value is "{searchParams.get("value")}".</p>
<button onClick={() => setSearchParams({})}>Click</button>
</div>
);
}

act(() => {
ReactDOM.createRoot(node).render(
<MemoryRouter initialEntries={["/search?value=initial"]}>
<Routes>
<Route path="search" element={<SearchPage />} />
</Routes>
</MemoryRouter>
);
});

let button = node.querySelector<HTMLInputElement>("button")!;
expect(button).toBeDefined();

expect(node.innerHTML).toMatch(/The current value is "initial"/);

act(() => {
button.dispatchEvent(new Event("click", { bubbles: true }));
});

expect(node.innerHTML).toMatch(/The current value is ""/);
});
});
14 changes: 8 additions & 6 deletions packages/react-router-dom/dom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,15 +88,17 @@ export function createSearchParams(

export function getSearchParamsForLocation(
locationSearch: string,
defaultSearchParams: URLSearchParams
defaultSearchParams: URLSearchParams | null
) {
let searchParams = createSearchParams(locationSearch);

for (let key of defaultSearchParams.keys()) {
if (!searchParams.has(key)) {
defaultSearchParams.getAll(key).forEach((value) => {
searchParams.append(key, value);
});
if (defaultSearchParams) {
for (let key of defaultSearchParams.keys()) {
if (!searchParams.has(key)) {
defaultSearchParams.getAll(key).forEach((value) => {
searchParams.append(key, value);
});
}
}
}

Expand Down
7 changes: 6 additions & 1 deletion packages/react-router-dom/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -853,13 +853,17 @@ export function useSearchParams(
);

let defaultSearchParamsRef = React.useRef(createSearchParams(defaultInit));
let hasSetSearchParamsRef = React.useRef(false);

let location = useLocation();
let searchParams = React.useMemo(
() =>
// Only merge in the defaults if we haven't yet called setSearchParams.
// Once we call that we want those to take precedence, otherwise you can't
// remove a param with setSearchParams({}) if it has an initial value
getSearchParamsForLocation(
location.search,
defaultSearchParamsRef.current
hasSetSearchParamsRef.current ? null : defaultSearchParamsRef.current
),
[location.search]
);
Expand All @@ -870,6 +874,7 @@ export function useSearchParams(
const newSearchParams = createSearchParams(
typeof nextInit === "function" ? nextInit(searchParams) : nextInit
);
hasSetSearchParamsRef.current = true;
navigate("?" + newSearchParams, navigateOptions);
},
[navigate, searchParams]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,30 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`useSearchParams allows removal of search params when a default is provided 1`] = `
<View>
<Text>
The current query is "
initial
".
</Text>
<View>
Click
</View>
</View>
`;

exports[`useSearchParams allows removal of search params when a default is provided 2`] = `
<View>
<Text>
The current query is "
".
</Text>
<View>
Click
</View>
</View>
`;

exports[`useSearchParams reads and writes the search string (functional update) 1`] = `
<View>
<Text>
Expand Down
40 changes: 40 additions & 0 deletions packages/react-router-native/__tests__/search-params-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ describe("useSearchParams", () => {
return <View>{children}</View>;
}

function Button({ children }: { children: React.ReactNode; onClick?: any }) {
return <View>{children}</View>;
}

it("reads and writes the search string", () => {
function SearchPage() {
let [searchParams, setSearchParams] = useSearchParams({ q: "" });
Expand Down Expand Up @@ -112,4 +116,40 @@ describe("useSearchParams", () => {

expect(renderer.toJSON()).toMatchSnapshot();
});

it("allows removal of search params when a default is provided", () => {
function SearchPage() {
let [searchParams, setSearchParams] = useSearchParams({
value: "initial",
});

return (
<View>
<Text>The current query is "{searchParams.get("value")}".</Text>
<Button onClick={() => setSearchParams({})}>Click</Button>
</View>
);
}

let renderer: TestRenderer.ReactTestRenderer;
TestRenderer.act(() => {
renderer = TestRenderer.create(
<NativeRouter initialEntries={["/search?value=initial"]}>
<Routes>
<Route path="search" element={<SearchPage />} />
</Routes>
</NativeRouter>
);
});

expect(renderer.toJSON()).toMatchSnapshot();

let button = renderer.root.findByType(Button);

TestRenderer.act(() => {
button.props.onClick();
});

expect(renderer.toJSON()).toMatchSnapshot();
});
});
14 changes: 9 additions & 5 deletions packages/react-router-native/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -288,16 +288,19 @@ export function useSearchParams(
defaultInit?: URLSearchParamsInit
): [URLSearchParams, SetURLSearchParams] {
let defaultSearchParamsRef = React.useRef(createSearchParams(defaultInit));
let hasSetSearchParamsRef = React.useRef(false);

let location = useLocation();
let searchParams = React.useMemo(() => {
let searchParams = createSearchParams(location.search);

for (let key of defaultSearchParamsRef.current.keys()) {
if (!searchParams.has(key)) {
defaultSearchParamsRef.current.getAll(key).forEach((value) => {
searchParams.append(key, value);
});
if (!hasSetSearchParamsRef.current) {
for (let key of defaultSearchParamsRef.current.keys()) {
if (!searchParams.has(key)) {
defaultSearchParamsRef.current.getAll(key).forEach((value) => {
searchParams.append(key, value);
});
}
}
}

Expand All @@ -310,6 +313,7 @@ export function useSearchParams(
const newSearchParams = createSearchParams(
typeof nextInit === "function" ? nextInit(searchParams) : nextInit
);
hasSetSearchParamsRef.current = true;
navigate("?" + newSearchParams, navigateOpts);
},
[navigate, searchParams]
Expand Down