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

Support for search #58

Closed
worudso opened this issue Jul 17, 2019 · 17 comments · Fixed by #267
Closed

Support for search #58

worudso opened this issue Jul 17, 2019 · 17 comments · Fixed by #267
Labels
V3 Features that won't be released in v2, but planned for the next major release

Comments

@worudso
Copy link

worudso commented Jul 17, 2019

In the latest version of wouter, when the location is somewebsite.com/users?name=john

const [location] = useLocation();

location only retrieves /users emitting ?name=john. And even though I push new query params with other methods, re-rendering is not triggered. But sometimes a view needs to depend on search params.

I think we have two options in a broad sense

  • add useSearch thing

    const [search, setSearch] = useSearch();
    search // "?name=john"

    or we can let wouter give searchParams as a URLSearchParams object, the parsed one.
    or how about useURL which gives current location as URL object?

  • let location be location.pathname + location.search.

    breaking change!

    In this case, we got a new problem to redefine routing rules. Should useRoute provide matching for search parameters? I don't think so. Routing rules can go the same just ignoring search.
    And the parsing for location.pathname + location.search is annoying. AFAIK, there is no builtin function to parse it. And even if there is, the code to parse would be repeated in many components.

@dpr-dev
Copy link

dpr-dev commented Jul 31, 2019

I think it is better to return a complex location object, as in RR

{
   pathname: string, 
   search: ''
}

const [location, push] = useLocation(); // Now we can get the search and the path from the location

@molefrog
Copy link
Owner

molefrog commented Aug 12, 2019

@worudso @dpr-dev Hey guys, thanks for the issue.
Been thinking about this a lot and what I had in mind is following:

// useLocation hook may return a third argument, which is basically a hash 
// of query params used; this is completely optional and apps that use 
// the hook already won't break
const [location, setLocation, queryParams] = useLocation();

// the signature of the useRoute hook stays the same, however it may return query params
// in the params hash along with dynamic segments
const [match, params] = useRoute("/app/users/:id");

// example: /app/users/alex?tab=settings
// -> { id: "alex", tag: "settings" }

In my opinion this can be one of the optimal solutions because:

  1. Doesn't break the existing API
  2. The query + route params merging technique is widely used, for example in Rails or Express (I guess).

Anyway, let me know what you think about this design?

@worudso
Copy link
Author

worudso commented Aug 13, 2019

This is my fork
https://github.com/ownerclan/wouter/blob/master/use-search-params.js
And I implemented useSearchParams for my immediate needs.

I'm not saying we'd better go useSearchParams. Instead, I'm saying @molefrog 's new useLocation might be the right choice. I hope my experiences that dealt with search params help you make a better decision.

The current version of my useSearchParams returns an array of [key, value] tuples. But for convenience, returning an object with keys of params is best. I had used that version of useSearchParams at first. It's like react's old state and setState which allows partial update of a state. This design had made my code looks better.

But there's a problem that there can be more than two params with the same name at once(This kind of usage actually exists in my project). And URLSearchParams object allows it. So we need to forget setState-like setSearchParams when we don't want to give an strong opionion about the usage of search params.

I think we'd hetter leave this problem(manipulating search params in an elegant way) to another utility library(lodash, immer, etc) and a programmer himself, and focus on conservative representation of URLSearchParams. But URLSearchParams object really sucks, I recommend to use an array of [key, value] tuples. We can map and filter and do another array operations on it while we can't with URLSearchParams.

And there's another problem that with useSearch thing we have multiple ways to update current url. It makes the behavior of setLocation and setSearchParams ugly.

setLocation can update path and search params and setSearchParams can only update search params and should preserve path.

So I 100% agree with new useLocation. But useRoute has a problem I metioned before. It can't be used with multiple keys with the same name. I agree that new useRoute can be useful in some case. But adding that feature in the future will not be breaking changes. How about keeping old simple useRoute behaviour and delaying changes for now?

@molefrog
Copy link
Owner

@worudso Not sure if I understand what you mean by:

there can be more than two params with the same name at once

You mean the search query might contain things like ?options[]=for&options[]=bar?

@worudso
Copy link
Author

worudso commented Aug 13, 2019

@molefrog yes, that's the case. Whether it's recommended or not, it's already supported by native URLSearchParams.

@dpr-dev
Copy link

dpr-dev commented Aug 13, 2019

@molefrog, very nice implementation

@ifokeev
Copy link

ifokeev commented Sep 4, 2019

@molefrog's implementation looks like what we need now. Looking for a "hash" property also https://www.w3schools.com/jsref/prop_loc_hash.asp

@kilianc
Copy link

kilianc commented Dec 8, 2019

Has this landed?

@molefrog
Copy link
Owner

molefrog commented Dec 9, 2019

@kilianc No, not yet 😞 There is a discussion going on in #102
The problem isn't in implementing this, but providing a good design that will enable us to keep the size low, plus will be generic enough.

@o-alexandrov
Copy link
Contributor

I think separating location.pathname and location.search tracking is the right approach.
Some components don't need to react on query string changes, while some do.
If the hook useLocation is going to track both, it will introduce performance regressions.

For now, to track search (query string changes) you could use:

I like the separation in react-use between:

  • useLocation
  • useSearchParam

I believe this library should do the same due to performance reasons and ability for a project to import only what's needed for their specific needs.

@MeloJR
Copy link

MeloJR commented Nov 16, 2020

Any news about this? It would be great to have access to the search map!

@ghost
Copy link

ghost commented Jan 2, 2021

I tried to use the useSearchParam hook from react-use but it was not reacting when using wouter's Link to navigate.

const getValue = (param) => new URLSearchParams(window.location.search).get(param)

function useSearchParam(param) {
    const [value, setValue] = useState(() => getValue(param))

    useEffect(() => {
        const onChange = () => setValue(getValue(param))

        window.addEventListener('popstate', onChange)
        window.addEventListener('pushstate', onChange)
        window.addEventListener('replacestate', onChange)

        return () => {
            window.removeEventListener('popstate', onChange)
            window.removeEventListener('pushstate', onChange)
            window.removeEventListener('replacestate', onChange)
        }
    }, [param])

    return value
}

This is because the events they are listening to are pushstate and replacestate instead of wouter's pushState and replaceState. I don't know who is correct here. Following the SO link from the wouter source code, looks like it should be pushstate and replacestate. It's also consistent with the popstate and other window events names.

I'm leaving this here in case someone else also runs into this problem when using react-use with wouter.

@ryanking1809
Copy link

ryanking1809 commented Jun 5, 2021

Correct me if I'm wrong but it looks like an update in regards to joaopaulobdac's issue above means a component will rerender if the query params change. So I believe you can just use URLSearchParams and long as you use useRoute or useLocation within the component. So you can make your own hooks using the following to wrap around the existing wouter api.

import { useLocation as useWouterLocation, useRoute as useWouterRoute } from "wouter";

export const useLocation = () => {
    const [location, setLocation] = useWouterLocation();
    return [location, setLocation, window.location.search];
};
// const [location, setLocation, search] = useLocation();

export const useRoute = (pattern) => {
    let [match, params] = useWouterRoute(pattern);
    if (match) {
        const urlSearchParams = new URLSearchParams(window.location.search);
        const queryParams = Object.fromEntries(urlSearchParams.entries());
        // params and queryParams can have the same name
        // this preferences params in that scenario
        params = {
			...queryParams,
			...params,
		};
    }
    return [match, params];
}
// const [match, params] = useMatch(pattern);

@stefensuhat
Copy link

is it possible to add search dynamically?

https://codesandbox.io/s/wouter-demo-nested-routes-forked-qmg6q?file=/src/index.js

when pressed add query it will add query but remove previous params.

how to test:
Press "Help Center" and navigate to topics. url will shown help/topics and pressed add query it will remove /topics not extending the url.

@HansBrende
Copy link
Contributor

Copying and pasting my response from #177:

The correct way to do this would be something like the example shown here: #232
Tbh, not sure why search changing is causing re-rendering by default... that seems misguided. Since there's currently no subscribe function exported, could easily write our own:

const events = ["popstate", "pushState", "replaceState", "hashchange"];
function subscribe(callback: () => void) {
    for (const event of events) {
         window.addEventListener(event, callback);
    }
    return () => {
        for (const event of events) {
            window.removeEventListener(event, callback);
        }
    }
}

function getCurrentValueOfMyParam() {
    return someFunctionOf(window.location);
}

And now we can simply do...

const myParam = React.useSyncExternalStore(subscribe, getCurrentValueOfMyParam);

No re-rendering if myParam hasn't changed, even if a million other things in the url have!

(Notice: no mucking with Router is necessary at all!)

@SalahAdDin
Copy link

Any solution ?

@molefrog molefrog added V3 Features that won't be released in v2, but planned for the next major release query strings and removed feature labels Nov 1, 2022
@coder-ka
Copy link

It works for me.

import React from "react";
import ReactDOM from "react-dom/client";
import { Router } from "wouter";
import useBrowserLocation from "wouter/use-location";
import App from "./App";

ReactDOM.createRoot(document.getElementById("root") as HTMLElement).render(
  <React.StrictMode>
    <Router
+      hook={(options) => {
+        const { pathname, search } = new URL(location.href);
+
+        const [_, nav] = useBrowserLocation(options);
+
+        return [pathname + search, nav];
+      }}
    >
      <App />
    </Router>
  </React.StrictMode>
);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
V3 Features that won't be released in v2, but planned for the next major release
Projects
None yet
Development

Successfully merging a pull request may close this issue.