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

Add forceRefresh option to createBrowserHistory #95

Closed
mjackson opened this issue Oct 12, 2015 · 11 comments
Closed

Add forceRefresh option to createBrowserHistory #95

mjackson opened this issue Oct 12, 2015 · 11 comments
Labels

Comments

@mjackson
Copy link
Member

Some users need a history that:

  • has clean URLs, like browser history
  • uses full page refreshes instead of pushState

See also remix-run/react-router#2243

@taion
Copy link
Contributor

taion commented Oct 12, 2015

The other option would be to just make this an option on createBrowserHistory, which might be easier all-around.

@jlongster
Copy link
Contributor

Oh, so if the browser doesn't support pushState it'll do full refreshes? Maybe in createBrowserHistory we can just pass an option to only do full refreshes? I like consolidating the history types based on schemes, not behavior. And in an ideal world there would be no difference to pushState and a full refresh, but as mentioned in the other issue this breaks when integrating with non-React systems (that mutate the DOM and require full refreshes).

I'm open to figuring out a way to do this in userland, if the demand for it isn't enough to warrant the feature (I know how much a feature costs, maintenance/docs/etc).

@jlongster
Copy link
Contributor

@taion you beat me by a few seconds :p

@mjackson
Copy link
Member Author

Good point, @taion. Let's try that first: adding an forceRefresh option to createBrowserHistory.

@mjackson mjackson changed the title Add createRefreshHistory Add forceRefresh option to createBrowserHistory Oct 12, 2015
@jlongster
Copy link
Contributor

That would solve everything!

@taion
Copy link
Contributor

taion commented Oct 12, 2015

@jlongster

BTW, because every issue is actually the same issue, this ties into remix-run/react-router#2211.

Adding something like forceRefresh to History.createBrowserHistory requires a minor version bump for history per semver; if we were to re-export that function as ReactRouter.createBrowserHistory, then we'd have to also cut a new release of react-router to make that functionality available, even though nothing in React Router itself will have changed.

@jlongster
Copy link
Contributor

@taion It depends on how you view react-router; I viewed it as a project that literally wrapped history so it has changed if you do that, and I'd be fine bumping the version. People bump project versions all the time when updating dependencies.

But no need to go into it here :) I'm fine with the decision in there.

@timdorr
Copy link
Member

timdorr commented Oct 12, 2015

Maybe call it refreshState to keep it inline with the other state modification methods.

@mjackson
Copy link
Member Author

It's not a method, @timdorr. It's an option to the create step.

let history = createBrowserHistory({ forceRefresh: true })

@timdorr
Copy link
Member

timdorr commented Oct 12, 2015

Whooooops! Nevermind me 😄

pwmckenna pushed a commit to pwmckenna/history that referenced this issue Oct 14, 2015
add support for forceRefresh

addresses issue: remix-run#95
pwmckenna pushed a commit to pwmckenna/history that referenced this issue Oct 14, 2015
@mjackson
Copy link
Member Author

Resolved in #100

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

No branches or pull requests

4 participants