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

connectWithRouter #54

Closed
wallzero opened this issue Jun 1, 2017 · 10 comments
Closed

connectWithRouter #54

wallzero opened this issue Jun 1, 2017 · 10 comments

Comments

@wallzero
Copy link
Contributor

wallzero commented Jun 1, 2017

I suggest a connect component for ui-router-react be provided since accessing context directly isn't ideal. Something like:

import React, {Component} from 'react';
import PropTypes from 'prop-types';

const ConnectWithRouter = (params = (router) => ({router})) => (WrappedComponent) => {
  class connector extends Component {
    static contextTypes = {
      router: PropTypes.object
    };

    constructor(props, context) {
      super(props, context);
      this.context = context;
    }

    render() {
      return <WrappedComponent {...params(this.context.router)} {this.props} />
    }

    return connector;
  }
}

return ConnectWithRouter;

Sorry I'd make a PR but I am not familiar enough with TypeScript.

@elboman
Copy link
Member

elboman commented Jun 5, 2017

Why do you feel you would need to access the context? Do you need the router instance?

Because you can always bootstrap the router manually and pass the instance to the UIRouter component. You can also access the router via the transition object that is injected via prop by the UIView component.

@wallzero
Copy link
Contributor Author

wallzero commented Jun 6, 2017

I can use the instance passed from UIView, but that becomes inconvenient if the component which requires router is deeply nested. Also, whether I bootstrap the router myself or not, the instance is still passed in context and I can use the above ConnectWithRouter.

The above HOC is really to act somewhat like dependency injection. My components can remain unaware of UIRouter and are simply passed predefined functions which are responsible for interacting with the router instance. It's the same pattern as react-redux's connect.

Including ConnectWithRouter or something similar is simply a convenience for users. They can always write it themselves.

@nateabele
Copy link
Member

I think I understand what you're saying. Maybe a better question is: is there something specific you need to access the router instance for that isn't provided by the existing state/view management APIs? Thanks.

@mhemrg
Copy link

mhemrg commented Jun 6, 2017

I think it is necessary, too. I implemented it in my project but it's better to find it in the main package.

@nateabele
Copy link
Member

Okay, can you guys please explain some use cases so we can understand how best to approach the problem? Thanks.

@mhemrg
Copy link

mhemrg commented Jun 15, 2017

So let's imagine that you have some nested components and your root component is connected to router via UIView and it has access to transition object. But how about the child components? You have to pass the object through your components and it is inconvenient.

@wallzero
Copy link
Contributor Author

wallzero commented Apr 2, 2018

Several months later and I think I am becoming proficient with TypeScript. :-)

I have an updated connectWithRouter HOC written in TypeScript:

import {UIRouterReact} from '@uirouter/react';
import PropTypes from 'prop-types';
import React, {
  Component,
  ComponentType
} from 'react';

// Diff / Omit taken from:
// https://github.com/Microsoft/TypeScript/issues/12215#issuecomment-311923766
// Native `Exclude` can replace `Diff` in `typescript@^2.8.0`
export type Diff<T extends string, U extends string> =
  ({ [P in T]: P } & { [P in U]: never } & { [x: string]: never })[T];
export type Omit<T, K extends keyof T> = Pick<T, Diff<keyof T, K>>;

export const defaultUpdate = (router: UIRouterReact, forceUpdate: () => void) => (
  router.transitionService.onSuccess({},() => (forceUpdate()))
);

export default <TRouterProps, TNeedsProps extends {}>(
  params: (
    router?: UIRouterReact,
    ownProps?: TNeedsProps
  ) => TRouterProps = (
    router: UIRouterReact,
    ownProps
  ) => ({
    ...ownProps as {},
    router
  } as any),
  update: false | typeof defaultUpdate = defaultUpdate
) => (
  <P extends TRouterProps, C extends {router: UIRouterReact}>(
    WrappedComponent: ComponentType<P>
  ) => (
    class Connector extends Component<
      Omit<P, keyof TRouterProps> & TNeedsProps,
      Omit<P, keyof TRouterProps> & TNeedsProps
    > {
      public static contextTypes = {
        router: PropTypes.object
      };

      public constructor(props: P & TNeedsProps, context: C) {
        super(props, context);
        this.context = context;

        this.state = params(this.context.router, this.props as any) as any;

        if (update) {
          update(this.context.router, () => {
            this.setState(params(this.context.router, this.props as any));
            return this.forceUpdate();
          });
        }
      }

      public render() {
        return (
          <WrappedComponent
            {...this.state}
            {...this.props}
          />
        );
      }
    }
  )
);

In addition to the params function accepting router, it now also accepts ownProps similar to react-redux in order to allow returning router props that change depending on the props initially passed.

Also, I ran into an issue where I would need the component to receive the latest values from the router, but a shallow compare of render doesn't trigger an update. To resolve this I added a second optional update function. It defaults to triggering a forceUpdate() on every state transition but can be updated with a more finely tuned function or disabled.

Soon I will be exporting connectWithRouter in ui-router-react-digest. I could open a PR if interested, but with react@^16.3.0 and the new context API, this may be obsolete in the future in favor of a simpler component. I will try out the new context API once the react type definitions are updated.

EDIT: HOC now caches props in state for more fine tuned control of updates.

@nateabele
Copy link
Member

@schlesiger You may want to fix your readme. It says npm install react-swipeable-views.

@wallzero
Copy link
Contributor Author

wallzero commented Apr 2, 2018

Whoops that's embarrassing. I'll update it along with these changes. Thanks!

@elboman
Copy link
Member

elboman commented Apr 5, 2018

Yeah, I was thinking that with the new context API we should get the connectWithRouter functionality for free, by simply consuming the context that will provide the router instance.

elboman added a commit that referenced this issue Apr 15, 2018
feat(UIRouterConsumer): add new `<UIRouterConsumer>` component to access the router instance via the new context API.

example:
```jsx
import {UIRouterConsumer} from '@uirouter/react';

class SomeComponent extends React.Component {
  render () {
    const router = this.props.router;
    // use the router via props
    return <div>whatever</div>
  }
}

export default () =>
  <UIRouterConsumer>
    {router => <SomeComponent router={router} />}
  </UIRouterConsumer>
```

BREAKING CHANGE: `@uirouter/react` now uses the new React 16.3 Context API. If you were accessing the router instance via the legacy context api (which was never explecitly supported) you need to update your code accordingly:

before:
```jsx
class SomeComponent extends React.Component {
  static contextTypes = {
    router: PropTypes.object
  }

  render () {
    // access context via this.context
    const router = this.context.router;
    // do whatever needed with the router
  }
}
```

after:
```jsx
class SomeComponent extends React.Component {
  render () {
    // access router via props
    const router = this.props.router;
    // do whatever needed with the router
  }
}

// when rendering the component wrap it with the `<UIRouterConsumer>` component
<UIRouterConsumer>
  {router => <SomeComponent router={router} />}
</UIRouterConsumer>
```

Closes #54
elboman added a commit that referenced this issue Apr 15, 2018
feat(UIRouterConsumer): add new `<UIRouterConsumer>` component to access the router instance via the new context API.

example:
```jsx
import {UIRouterConsumer} from '@uirouter/react';

class SomeComponent extends React.Component {
  render () {
    const router = this.props.router;
    // use the router via props
    return <div>whatever</div>
  }
}

export default () =>
  <UIRouterConsumer>
    {router => <SomeComponent router={router} />}
  </UIRouterConsumer>
```

BREAKING CHANGE: `@uirouter/react` now uses the new React 16.3 Context API. If you were accessing the router instance via the legacy context api (which was never explecitly supported) you need to update your code accordingly:

before:
```jsx
class SomeComponent extends React.Component {
  static contextTypes = {
    router: PropTypes.object
  }

  render () {
    // access context via this.context
    const router = this.context.router;
    // do whatever needed with the router
  }
}
```

after:
```jsx
class SomeComponent extends React.Component {
  render () {
    // access router via props
    const router = this.props.router;
    // do whatever needed with the router
  }
}

// when rendering the component wrap it with the `<UIRouterConsumer>` component
<UIRouterConsumer>
  {router => <SomeComponent router={router} />}
</UIRouterConsumer>
```

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

No branches or pull requests

4 participants