Skip to content

Conversation

gmaclennan
Copy link
Contributor

history={HashHistory} should be history = {new HashHistory()}

history={HashHistory} should be history = {new HashHistory()}
@gmaclennan gmaclennan changed the title fix history={HashHistory} fix docs history={HashHistory} Jun 18, 2015
@Schniz
Copy link
Contributor

Schniz commented Jun 18, 2015

👍

ryanflorence added a commit that referenced this pull request Jun 18, 2015
fix docs history={HashHistory}
@ryanflorence ryanflorence merged commit 8b1a278 into remix-run:master Jun 18, 2015
@Schniz
Copy link
Contributor

Schniz commented Jun 23, 2015

Now that I think of it, I find calling new HashHistory in the Router props kinda misleading. especially if you use libraries like microcosm that wrap your Router calls:
Reinitiating an object inside the render method could cause a mess in the diffing. and in this example, it actually throws an error in the router if you pass a new HashHistory object..

thoughts?

@ryanflorence
Copy link
Member

Yeah, I'd rather we had singletons for folks to use.
On Tue, Jun 23, 2015 at 3:39 PM Gal Schlezinger notifications@github.com
wrote:

Now that I think of it, I find calling new HashHistory in the Router
props kinda misleading. especially if you use libraries like microcosm
https://github.com/vigetlabs/microcosm that wrap your Router calls:
Reinitiating an object inside the render method could cause a mess in the
diffing. and in this example, it actually throws an error in the router if
you pass a new HashHistory object..

thoughts?


Reply to this email directly or view it on GitHub
#1339 (comment).

Schniz added a commit to Schniz/react-router that referenced this pull request Jun 27, 2015
…'react-router/lib/HashHistory'`

this is an implementation for what @ryanflorence said on remix-run#1339 and @mjackson on remix-run#1426.
@Schniz Schniz mentioned this pull request Jun 27, 2015
@lock lock bot locked as resolved and limited conversation to collaborators Jan 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants