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

Discuss about the state pass into Link or navigate #102

Closed
cbbfcd opened this issue Nov 20, 2019 · 20 comments
Closed

Discuss about the state pass into Link or navigate #102

cbbfcd opened this issue Nov 20, 2019 · 20 comments
Labels
V3 Features that won't be released in v2, but planned for the next major release

Comments

@cbbfcd
Copy link
Contributor

cbbfcd commented Nov 20, 2019

Maybe we also need another way to pass parameters.

just like this:

const [, navigator] = useLocation();
navigate(path: string, replace?: boolean, state?: object);

// and
<Link to='/app' state={state}/>

@omgovich @molefrog

@molefrog
Copy link
Owner

I've been thinking about adding a more generic prop called navigate with navigation options that can be passed down to the navigation method. The motivation is simple: it might be possible that you're using a different location hook (like hash-based location) where settings like this don't make sense. And that could also allow us to specify replace option as well. Basically wouter will know nothing about the format of options that the current location hook is using:

<Link to='/app' navigate={{ replace: true, state: { animated: true } }} />
// calls navigate({ replace: true, state: { .. })

This will, however, require us to change the interface of the navigate method to keep the backward compat, but we could of course process true/false as an edge case and convert it to options internally:

const navigate = (to, options = {}) => {
  if (typeof options === 'boolean') options = { replace: options }
  const { replace = false, state = null } = options

  ...
}

But of course, that's a bit size-consuming.

@molefrog
Copy link
Owner

molefrog commented Nov 21, 2019

But the biggest question here is how are we going to return current history.state back to the user? One of the ideas I had is when useRoute is called merge this state in params.

const [match, params] = useRoute("/users/:id")

// when called with /users/1 and navigation performed with `history.state` = { animated: true }
// the result is:
match // true
params // { id: 1, animated: true } 

Which of course a bit hacky and conventional, but on the other hand will allow us to also merge query params later on. And it allow us to keep the semantics. I would rather have this than:

const [match, params, state, query] = useRoute("/users/:id")
// looks complicated...

Well, the last option is to leave it up to the user:

const [match, params] = useRoute(...)
const state = history.state || {} 

// params isn't polluted with the state in this case

@cbbfcd
Copy link
Contributor Author

cbbfcd commented Nov 21, 2019

For scalability and compatibility, I should think of a way to replace history.state, I will do a search.

@destructobeam
Copy link

destructobeam commented Nov 21, 2019

Just my opinion, but in terms of the Link component from a usage perspective, I think I prefer @soulchainer’s idea of replace being a bool with an optional state object prop, e.g.

<Link to="/" replace state={{ animate: true }} />

As for the hooks API the state and params would be seperate use cases no? So something like:

// Matching path params
const [match, params] = useRoute("/users/:id")

// Get state
const [location, setLocation, state] = useLocation()

// With setLocation signature something like:
setLocation(to: String, replace: Bool = false, state?: Object)

And then possibly just leave a search params hook in its own file, as it’s generally not going to be associated with "matching" a route, and it’s not really part of history API that useLocation wraps.

import useSearchParams from 'wouter/search-params'

// Minimal implementation
const search = useSearchParams()

// With setSearch convenience method
const [search, setSearch] = useSearchParams()

With search ideally being a plain object. Could also possibly allow passing down a custom parser function in context if needed? setSearch here being just a potential shortcut to update the search params in the address bar, could be left out if out of scope.

Off the top of my head those seem like okay trade offs, and more closely match up with their respective browser APIs?

Edits: Fleshed out some ideas/examples.

@cbbfcd
Copy link
Contributor Author

cbbfcd commented Nov 21, 2019

<Link to="/" replace state={{ animate: true }} /> 👍

@tonyhb
Copy link

tonyhb commented Jan 3, 2020

The approach by @destructobeam in #102 (comment) sounds like a really good path forward.

@o-alexandrov
Copy link
Contributor

I think wouter shouldn't do any state management.
Users of this library could already:

  • optionally handle state using any state manager of their choice in the familiar to them pattern

@molefrog molefrog mentioned this issue Jul 1, 2020
@molefrog
Copy link
Owner

molefrog commented Jul 7, 2020

@o-alexandrov Thanks, I absolutely agree with the point!

I do think that the state support (as well as the search support) can be added through extensions. I already expressed my ideas in #108 (comment)

@cbbfcd There isn't a huge list of new features I'd like to add to wouter. The main reason is it would be nice to maintain the > current state of the library size and its capabilities.

There are things however, that our users request quite a lot: like query params etc. For me that was especially tricky to
decide if I want to just stop adding new things, or start extending the library heavily.

After giving it some thoughts while I was recovering from my burn out last year (which was the main reason I stopped
working actively on it), I've decided that wouter should support new features through external extendability. I really like
that you can currently specify your custom useLocation hook, though not a lot of people are aware of that. But I really
want to develop this idea further: wouter is minimalistic by default, but it's extendable, so if you need an extra feature, you
can require a separate package for that.

I was already able to implement most of the ideas here: #121

With the update above the state support could be as simple as following:

import useLocationWithState from '@wouter/use-location-state';

const App = () => (
  <Router hook={useLocationWithState}>
    <Link to="/app" state={{ foo: 'bar' }}>Click Me</Link>
  </Router>
);

@destructobeam
Copy link

@o-alexandrov The suggestion wasn’t for wouter to manage state, but it already is an interface to the history API which itself has state, so the idea was to read/write to that.

That said, a hook that would provide that seems like a good approach.

@cbbfcd
Copy link
Contributor Author

cbbfcd commented Oct 22, 2020

i will try to make a pr for it

@molefrog
Copy link
Owner

@cbbfcd Ok!
Can we just discuss one thing beforehand? Here is the list of ideas/questions I have regarding this feature:

  • Should this live in the same repo as wouter or be a standalone repo or package?
  • Should the new location also support other requested features like search? I mean we could create a package called wouter-location that exports a useLocation function but on steroids: with search, state and other features if needed. The whole thing could be written in TS to simplify the type export.

@cbbfcd
Copy link
Contributor Author

cbbfcd commented Oct 22, 2020

@molefrog Thanks for your reply.
Fully agree to implement related functions in a new repo. similar to the plug-in mechanism to enhance wouter, cool!!

🆒 👇

import useLocation from '@wouter/location'

so, the first features we need to implement in a beta version:

  • support state
  • support search

any supplement ?

@cbbfcd
Copy link
Contributor Author

cbbfcd commented Oct 22, 2020

may be we can support hash router too.

import useLocation from '@wouter/location'

// e.g. history default true, but if the browser does not support hash or set history false, we use hash router
useLocation({ history: true })

@cbbfcd
Copy link
Contributor Author

cbbfcd commented Oct 23, 2020

maybe an interceptor should be supported too in new useLocation.

const interceptor = (currentPath, nextPath) => {
    if(confirm('Do you want to leave?')){
        return nextPath;
    }
    return currentPath;
}

useLocation({ interceptor })

@omgovich
Copy link
Contributor

omgovich commented Oct 23, 2020

Should we maybe find a way to allow multiple "patching"?
I mean something like high order functions:

import { withState, withSearch } from '@wouter/addonds'

const useAdvancedLocation = withState(withSearch(useLocation))

@tony-scio
Copy link

Just curious if this is still planned. My project uses it with react-router and it's the only thing I haven't figured out how to convert to let us switch to wouter.

Does anyone have a good pattern for a workaround?

@cbbfcd
Copy link
Contributor Author

cbbfcd commented Sep 22, 2021

@tony-scio hi, 👋 ~

There is currently no best way to convince everyone. I think the simplest implementation is to pass the state through navigate

const navigate = useCallback(
    (to, { replace = false, state = {} } = {}) =>
      history[replace ? eventReplaceState : eventPushState](
       { state, key: createKey() },
        "",
        // handle nested routers and absolute paths
        to[0] === "~" ? to.slice(1) : base + to
      ),
    [base]
  );

and then mix it into the props in the Route component.

// ...
const state = getStateFromHistory();
if (component) return h(component, { params, state });

  // support render prop or plain children
  return typeof children === "function" ? children({...params, ...state}) : children;

as a workaround, you can control the history state in a store, like context, it's not the best way, but should work well.

@SalahAdDin
Copy link

Just my opinion, but in terms of the Link component from a usage perspective, I think I prefer @soulchainer’s idea of replace being a bool with an optional state object prop, e.g.

<Link to="/" replace state={{ animate: true }} />

As for the hooks API the state and params would be seperate use cases no? So something like:

// Matching path params
const [match, params] = useRoute("/users/:id")

// Get state
const [location, setLocation, state] = useLocation()

// With setLocation signature something like:
setLocation(to: String, replace: Bool = false, state?: Object)

And then possibly just leave a search params hook in its own file, as it’s generally not going to be associated with "matching" a route, and it’s not really part of history API that useLocation wraps.

import useSearchParams from 'wouter/search-params'

// Minimal implementation
const search = useSearchParams()

// With setSearch convenience method
const [search, setSearch] = useSearchParams()

With search ideally being a plain object. Could also possibly allow passing down a custom parser function in context if needed? setSearch here being just a potential shortcut to update the search params in the address bar, could be left out if out of scope.

Off the top of my head those seem like okay trade offs, and more closely match up with their respective browser APIs?

Edits: Fleshed out some ideas/examples.

There is no wouter-search yet...

@cbbfcd
Copy link
Contributor Author

cbbfcd commented Jul 8, 2022

I've been busy lately, I'll try to provide an available

@molefrog molefrog added the V3 Features that won't be released in v2, but planned for the next major release label Sep 14, 2023
This was referenced Sep 27, 2023
@molefrog
Copy link
Owner

Fyi, we added a full support for state navigation in v3.0.0 (see #346 and #349), you can now use v3.0.0-rc.1 to test it out.

Usage:

const [,navigate] = useLocation();

navigate("/", { state: "string" }) // state can be anything
navigate("/", { state: { foo: bar }})

<Link to="/" state={{ modal: true }} />
<Redirect to="/" state={{ modal: true }} />

Also, we have added a hook for hash navigation, and it does support state too:

import { useHashLocation } from "wouter/use-hash-location"

<Router hook={useHashLocation}>
  <Link to="/" state={{ modal: true }} />
</Router>

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

No branches or pull requests

8 participants