Skip to content

Link that only updates the query (this.updateQuery) #378

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

Closed
ryanflorence opened this issue Oct 9, 2014 · 14 comments
Closed

Link that only updates the query (this.updateQuery) #378

ryanflorence opened this issue Oct 9, 2014 · 14 comments

Comments

@ryanflorence
Copy link
Member

Very often you want a link that only updates one or a few parts of the query or params, having to reconstruct and then mutate the current stuff is a pain (I'll type up some real-world use-cases later, I'm in a bit of a hurry right now).

Perhaps we can do a couple things to <Link/> make this awesome:

Optional to

I know, we had this once and @machty is awesome.

If you leave off to in a link, it will just use the current route.

add setQuery and setParams

These would work like setState in a component, namely, it merges the current data with new data.

add replace option

Would be false by default, if true the router will use replaceWith instead of transitionTo.

Use Case

Imagine headers in a sortable table, when you click them, you want to update the query, but not all of it, and you don't actually care what route you're at, you just want to change sortBy. Finally, you don't want an entry in the history.

<th><Link setQuery={{sortBy: 'firstName'}} replace={true}>First Name</Link></th>
<th><Link setQuery={{sortBy: 'lastName'}} replace={true}>Last Name</Link></th>

WHATCHOOTHINK?

@ryanflorence
Copy link
Member Author

@ryankshaw

@machty
Copy link

machty commented Oct 9, 2014

WHAT. YOU DELETED MY WORK? UNACCEPTABLE

@ryanflorence
Copy link
Member Author

it broke stuff and we were so young

@machty
Copy link

machty commented Oct 9, 2014

ARMISTICE BREACHED. LAUNCH THE KVO9000

@ryanflorence
Copy link
Member Author

hahahahaha, I am actually laughing out loud.

@mjackson
Copy link
Member

mjackson commented Oct 9, 2014

I think the minimum API we need here is:

  1. <Link> w/o to and
  2. <Link replace>

setQuery can be done using <Link query={ merge(this.props.query, { some: "stuff" }) }> or just <Link query={{ some: "stuff" }}> if you want to replace the entire query on the current route.

setParams would actually change portions of window.location.pathname, so it doesn't make sense to me. Just use <Link params={{ some: "param" }}> instead.

setQuery and setParams do kind of make sense as helper methods for programmatically updating the URL, but that's what transitionTo and replaceWith are for.

@ryanflorence
Copy link
Member Author

setQuery can be done using <Link query={ merge(this.props.query, { some: "stuff" }) }>

now that we have this.getActiveQuery I think you're right.

@ryankshaw
Copy link
Contributor

right, so with getActiveQuery you could mixin ReactRouter.ActiveState and do
<Link query={merge(this.getActiveQuery(), {some: 'stuff'}}
and not have to pass props.query all the way down to the ColumnHeader component.

+1 to <Link> w/o to

@mjackson
Copy link
Member

you don't even need getActiveQuery. you already have the query in this.props.query.

Edit: oh, right. you're talking about further down the component hierarchy. Got it now :)

@ryankshaw
Copy link
Contributor

so question:
before, I had something like:

ReactRouter.Link({
  query: merge(this.props.query, { ascending: !this.props.query.ascending })
  className: (this.props.query.ascending ? 'ascending' : 'descending')
}, 'this column is being sorted ' + (this.props.query.ascending ? 'ascending' : 'descending'))

but if i do:

mixins: [ReactRouter.ActiveState]
...
ReactRouter.Link({
  query: merge(this.getActiveQuery(), {ascending: !this.getActiveQuery().ascending })
  className: (this.getActiveQuery().ascending ? 'ascending' : 'descending')
}, 'this column is being sorted ' + (this.getActiveQuery().ascending ? 'ascending' : 'descending'))

I am not guaranteed that my className or the "this column is being sorted descending" text will stay up to date right? Because in the first, since I was getting the query in this.props, I knew that render would be re-run if the query string changed. But in the latter, since I am not getting it passed in as a prop, it won't cause a re-render right? Or is that fact that a re-render was called on a parent good enough to also ensure that my render gets called

so if you want to re-render whenever the url query string changes, you still need to pass this.props.query all the way down right?

@ryanflorence
Copy link
Member Author

Edit: oh, right. you're talking about further down the component hierarchy. Got it now :)

actually, further up.

A persistent toolbar has a preview button that needs the query params from a deeply nested route to know what to preview.

@mrmurphy
Copy link

Hi, friends! I'm feeling the pain right now of building a whole link just to set a query string. Has this ticket taken a back seat?

@mrmurphy
Copy link

mrmurphy commented Feb 5, 2015

Welp, I don't really know what I'm doing, but here's a pull request!

#773

Feel free to tell me that I don't know what I'm doing! 🐫 🎆 🍃

scothis added a commit to scothis/react-router that referenced this issue Jun 5, 2015
It's often desirable to transition to a new state changing only a single
part of the URL. This is currently cumbersome as Router.transitionTo and
Router.replaceWith both require the full state of the new URL.

Router#transitionToMixin and Router#replaceWithMixin are identical to
their non-Mixin siblings, however, they use the current router state,
augmenting it with the provided values.

These new methods are also exposed on the Navigation mixin.

For example using the current Navigation and State mixins:

    // before
    var query = this.getQuery();
    query.sortDir = 'desc';
    this.replaceWith(
      this.getPathname(),
      this.getParams(),
      query
    );

    // after
    this.replaceWithMixin(null, null, { sortDir: 'desc' });

Issue: remix-run#378
@marnusw
Copy link

marnusw commented Jun 30, 2015

Earlier @mjackson said

setQuery and setParams do kind of make sense as helper methods for programmatically updating the URL, but that's what transitionTo and replaceWith are for.

I have the case where I'm using a <select> to switch between view states:

<Input type="select" value={this.props.query.groupBy} onChange={this.handleGroupBy}>
  <option value="category">View by Category</option>
  <option value="date">View by Date</option>
</Input>

and then in my handler I do:

handleGroupBy(ev) {
  // ...
  this.context.router.replaceWith(this.context.router.getCurrentPathname(), 
    this.props.params, {groupBy: value});
}

It seems like a lot of code just to update the query as opposed to

this.context.router.setQuery({groupBy: value});

Changing the path or params seems to imply a transition, but updating the query quite often only affects the layout/state of the current page. I would, therefore, argue that the setQuery method does have merit. I gather that the <Link> element can solve this issue, but changing the query programmatically would also be handy.

If there is a better way to do the above which I'm missing please let me know!

@lock lock bot locked as resolved and limited conversation to collaborators Jan 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants