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

Missing <Link> component? #297

Closed
manuelkiessling opened this issue Apr 21, 2019 · 11 comments
Closed

Missing <Link> component? #297

manuelkiessling opened this issue Apr 21, 2019 · 11 comments

Comments

@manuelkiessling
Copy link

This is not neccessarily a problem with connected-react-router - chances are that I'm just not "getting it".

I've got everything set up and working beautifully. However, working with onClick={() => push("/foo")} everywhere felt like a step back from just using <Link to="/foo"> from react-router. However, using react-router's Link component no longer works - the browser navigates, but the Redux state doesn't change.

Thus, I created my own Link component, like this:

import React, { Component } from "react";
import { connect } from "react-redux";
import { push } from "connected-react-router";

class Link extends Component {
    constructor(props) {
        super(props);
    }

    render() {
        return (
            <a href={this.props.href} onClick={ (e) => { e.preventDefault(); this.props.push(this.props.href); } } { ...this.props }>
                {this.props.children}
            </a>
        );
    }
}

export default connect(null, { push })(Link);

Is that a valid approach? Shouldn't such a component even be part of connected-react-router?

@chenyil
Copy link

chenyil commented Apr 23, 2019

However, using react-router's Link component no longer works - the browser navigates, but the Redux state doesn't change.

I'm in the exact same situation. I followed every step here https://github.com/supasate/connected-react-router#usage

@xesf
Copy link

xesf commented Apr 23, 2019

Usually when that happens it means that the history is not the same. Make sure you have the right setup on the history->store->provider->router

Anyway, I am still having troubles using the latest version of connected-react-route. I had to rollback to version 4.5.

@manuelkiessling
Copy link
Author

@xesf Wait, are you saying that react-router's Link component is actually supposed to work even if introducing connected-react-router into the mix?

@xesf
Copy link

xesf commented Apr 28, 2019

It always worked for me, even the old version.
I manage to upgrade all the versions now to the latest, expect reduz-saga and seems to be working just fine.

I have a component library that has the reference of Link from a Button component. The lib is then used in the apps which include the connected react router. I mainly use connected router to dispatch routes from sagas, while using Links in most of the cases from View components. I do have exceptions where from components I dispatch to router as well.

@zjr
Copy link

zjr commented Apr 28, 2019

I had this problem because I had a <BrowserRouter> nested under my <ConnectedRouter>, you actually only should have the latter.

@zjr
Copy link

zjr commented Apr 28, 2019

#230 (comment)

@chenyil
Copy link

chenyil commented Apr 29, 2019

@zjr thank u for pointing this out! I have my <ConnectedRouter> nested under a <BrowserRouter>, it's the other way around but I think it's basically the same problem.
I have to integrate with a react component wrapper that uses BrowserRouter, and the wrapper is a component, so I can't put a redux store in it. Does it mean I can't use connected-react-router as expected in my react/redux project?

@xesf
Copy link

xesf commented Apr 30, 2019

@chenyil, you will have to remove the BrowserRoute has pointed in step 3 of the readme and make sure you have a share history with store and connected router.
Actually if tou follow the steps from the readme it should be enough to get it working.

@chenyil
Copy link

chenyil commented Apr 30, 2019

Thank you, @xesf I can't removeBrowserRouter completely because I don't own that wrapper component. In addition to the setup step in the readme, I worked with the owner and came up with something like this. I hope this helps other devs who have to integrate with a component that already uses BrowserRouter

Wrapper component:

render() {
  const { children, customRouter, customLocation } = this.props
  const RouterComponent = customRouter ? 'div' : BrowserRouter

  return (
    <Link location={customLocation} to='/root/menu'>menu</NavLink>
    <RouterComponent>
      <Route path='/root' location={customLocation} component={children}>
    </RouterComponent> 
  )

App.js

const mapStateToProps = state => {
  return {
    location: state.router.location
  }
}
class App extends Component {
  render() {
    const { location } = this.props
    <Wrapper customRouter={true} customLocation={location}>
      <Content />
    </Wrapper>
  }
}

connect(mapStateToProps)(App)

@xesf
Copy link

xesf commented May 1, 2019

The best choice will be to ask for an extension of customRouter to be an outside component, instead of a default div, if you cannot access either App or Wrapper components.

@manuelkiessling
Copy link
Author

I no longer have the problem after upgrading to the latest version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants