{"action":"created","issue":{"url":"https://api.github.com/repos/ReactTraining/history/issues/478","repository_url":"https://api.github.com/repos/ReactTraining/history","labels_url":"https://api.github.com/repos/ReactTraining/history/issues/478/labels{/name}","comments_url":"https://api.github.com/repos/ReactTraining/history/issues/478/comments","events_url":"https://api.github.com/repos/ReactTraining/history/issues/478/events","html_url":"https://github.com/ReactTraining/history/issues/478","id":229891705,"number":478,"title":"Proposal: Make `search` friendlier","user":{"login":"pshrmn","id":1127037,"avatar_url":"https://avatars3.githubusercontent.com/u/1127037?v=3","gravatar_id":"","url":"https://api.github.com/users/pshrmn","html_url":"https://github.com/pshrmn","followers_url":"https://api.github.com/users/pshrmn/followers","following_url":"https://api.github.com/users/pshrmn/following{/other_user}","gists_url":"https://api.github.com/users/pshrmn/gists{/gist_id}","starred_url":"https://api.github.com/users/pshrmn/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pshrmn/subscriptions","organizations_url":"https://api.github.com/users/pshrmn/orgs","repos_url":"https://api.github.com/users/pshrmn/repos","events_url":"https://api.github.com/users/pshrmn/events{/privacy}","received_events_url":"https://api.github.com/users/pshrmn/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":3,"created_at":"2017-05-19T07:22:13Z","updated_at":"2017-05-19T19:45:43Z","closed_at":null,"body":"### The Problem\r\n\r\nCurrently, the `location` object uses `pathname`, `search`, and `hash` strings to create URIs. This makes the job really easy for `history`, but causes a lot of consternation\r\n\r\nIn `history`, v2/3, there is built-in (through enhancers) support for query parsing. The biggest issues with that is that 1. you end up with redundant data with the `search` and `query` properties and 2. you end up bundling query parsing code that may not even do what the user wants it to do.\r\n\r\n### The Proposal\r\n\r\nWhat I am proposing is that the `search` property of a location can be anything you want. It could be a string, or an object, or a function for all we care. The only important thing is that your `history` object knows how to derive a string from it for the URI. To do this, the history constructors should accept a new option: `stringify`.\r\n\r\n```js\r\nconst history = createBrowserHistory({\r\n  stringify: qs.stringify\r\n})\r\n```\r\n\r\nWhen `history` calls `createPath` to create the URI that will be displayed in the address bar, instead of calling:\r\n\r\n```js\r\npathname + search + hash\r\n```\r\nit would call\r\n```js\r\npathname + stringify(search) + hash\r\n```\r\n\r\nOf course, the `search` value should be consistent, so we will also need a way to create a location object with a URI that has the desired `search` property for the user. Thus, we should also add a `parse` option to the constructor.\r\n\r\n```js\r\nconst history = createBrowserHistory({\r\n  stringify: qs.stringify,\r\n  parse: qs.parse\r\n})\r\n```\r\n\r\nBasically, `parsePath` would be updated to call `parse` on the portion of the path that begins with `?` through to the end of the string.\r\n\r\n```js\r\nsearch = parse(pathname.substr(searchIndex))\r\n```\r\n\r\nDefault `stringify` and `parse` functions would just return the values that they pass, so by default `search` would expect to be a string.\r\n\r\n### Cons\r\n\r\n* This makes `history` more complicated.\r\n* `history` doesn't _really_ care about anything but how to turn location objects into URI strings and how to turn URI strings into location objects. `history` is predominantly a routing package, and query strings are not important for routing, so representing them as strings is good enough.\r\n* Despite all of the complaints and the code being open source, no one has submitted a PR with their solution to fix this. Makes you wonder how big of an issue this really is...\r\n\r\n### Pros\r\n\r\n* Convenience for developers\r\n* A dramatic reduction in requests for/complaints about this in the React Router issues. \r\n* As long as we can parse and stringify it, who cares what the `search` property is?\r\n\r\nWhile I have advocated that this isn't necessary, complaints keep rolling in, so I thought that someone should make a proposal for a solution to this. I should be able to submit a PR for this soon because the changes to the code are pretty minor. A number of tests will need to be added in order to verify that this actually works as expected, but I think that it should be fairly easy to implement."},"comment":{"url":"https://api.github.com/repos/ReactTraining/history/issues/comments/302794688","html_url":"https://github.com/ReactTraining/history/issues/478#issuecomment-302794688","issue_url":"https://api.github.com/repos/ReactTraining/history/issues/478","id":302794688,"user":{"login":"pshrmn","id":1127037,"avatar_url":"https://avatars3.githubusercontent.com/u/1127037?v=3","gravatar_id":"","url":"https://api.github.com/users/pshrmn","html_url":"https://github.com/pshrmn","followers_url":"https://api.github.com/users/pshrmn/followers","following_url":"https://api.github.com/users/pshrmn/following{/other_user}","gists_url":"https://api.github.com/users/pshrmn/gists{/gist_id}","starred_url":"https://api.github.com/users/pshrmn/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pshrmn/subscriptions","organizations_url":"https://api.github.com/users/pshrmn/orgs","repos_url":"https://api.github.com/users/pshrmn/repos","events_url":"https://api.github.com/users/pshrmn/events{/privacy}","received_events_url":"https://api.github.com/users/pshrmn/received_events","type":"User","site_admin":false},"created_at":"2017-05-19T19:45:43Z","updated_at":"2017-05-19T19:45:43Z","body":"I have a working (I think) implementation that you can see the changes required for it here: https://github.com/ReactTraining/history/compare/master...pshrmn:search-is-whatever. Only one breaking change, and that was out of convenience not necessity. You'll probably want to use the split view because git doesn't do a great job matching the actual changes in the unified view. Not going to PR it, but just throwing it out there.\r\n\r\n@mjackson Alternatively, any thoughts on rendering a component that adds a `query` property to the `location` object\r\n like this? https://gist.github.com/pshrmn/d0e888dae58856a95c87a80f730d61fc I don't think that that should break anything and would make the parsed query object available throughout the React application.\r\n\r\nAnyways, if you're taking a look at `history` things, I'm much more interested in #472 and #465."}}
{"actor":{"display_login":"pshrmn"}}