Skip to content

Conversation

Schniz
Copy link
Contributor

@Schniz Schniz commented Jun 27, 2015

Instead of just updating the docs, this is an implementation for what @ryanflorence said on #1339 and @mjackson on #1426.

this PR lets you use a history singleton with BrowserHistory and HashHistory:

var { history } = require('react-router/lib/HashHistory');
React.render(<Router history={ history } ... />, ...);

Schniz and others added 9 commits June 26, 2015 22:04
`new BrowserHistory()` shouldn't be inside the props decleration but outside the render method.
declaring the history outside of the render function
moved the history declaration outside of the `render` function.
…'react-router/lib/HashHistory'`

this is an implementation for what @ryanflorence said on remix-run#1339 and @mjackson on remix-run#1426.
@Schniz
Copy link
Contributor Author

Schniz commented Jun 27, 2015

@mjackson, I decided on calling it history instead of getSingleton.
Its less about implementation detail (that can be changed later), and more about what you get from this function

@Schniz Schniz changed the title WIP: History singletons History singletons Jun 27, 2015
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

People who need to support IE8 (poor folks) are going to gripe about this. Let's just export var history = new BrowserHistory in this module instead. Then people can still import { history } and we'll be fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@mjackson
Copy link
Member

Thanks for your work on this @Schniz !

@gaearon
Copy link
Contributor

gaearon commented Jun 27, 2015

💃

@Schniz
Copy link
Contributor Author

Schniz commented Jun 29, 2015

damn you IE 8.

@Schniz
Copy link
Contributor Author

Schniz commented Jun 29, 2015

@mjackson fixed this 👍

@Schniz
Copy link
Contributor Author

Schniz commented Jun 30, 2015

the tests don't pass for some reason, and don't seem to be mine. should I rebase for the current master?

@mjackson
Copy link
Member

@Schniz The only failing test is the one that tests scrolling, which @taurose fixed in c61a0f5. We're currently working on getting that branch merged, so this one should be good to go for now. When that branch merges we'll be green again.

Thanks!

mjackson added a commit that referenced this pull request Jun 30, 2015
@mjackson mjackson merged commit c1c8eb5 into remix-run:master Jun 30, 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