HOC inject example about mobx-react-lite HOT 29 CLOSED

I have improved the sandbox:

https://codesandbox.io/s/qx18xrrvmq

Now inject has a setStore function, doesn't need a React context and works like a HOC, but doesn't add the extra component.

This is the code:

let store = null;

export const setStore = newStore => {
  store = newStore;
};

const inject = selector => baseComponent => {
  const component = ownProps => {
    if (store === null)
      throw new Error('Please, use setStore before your ReactDOM.render call');
    return useObserver(() =>
      baseComponent({ ...ownProps, ...selector(store, ownProps) })
    );
  };
  component.displayName = baseComponent.name;
  return component;
};

export default inject;

These are the instructions:

Before your ReactDOM.render you need to set your store using setStore:

import store from './store';
import { setStore } from './mobx-react-inject';

setStore(store);

ReactDOM.render(<App />, document.getElementById('root'));

Then, just use inject in any component:

import inject from '../mobx-react-inject';

const NameDisplayer = ({ name }) => <h1>{name}</h1>

const InjectedNameDisplayer = inject(store => ({
    name: store.user.name
}))(NameDisplayer)

inject works like the old mobx-react inject (https://github.com/mobxjs/mobx-react#customizing-inject) with the custom mapper:

mapperFunction: (store, ownProps) => additionalProps

with the only difference that it doesn't add an extra component to your React tree.

I think it's pretty neat. It allows you to:

• Keep your components absolutely dumb. They only receive final props and just have to render.
• Avoid the use of observer(MyComponent) everywhere.
• Use named exports to unit test your dumb components, like this:

 // Named export for unit testing
export const NameDisplayer = ({ name }) => <h1>{name}</h1>

 // Default export to use in the app
export default inject(store => ({
    name: store.user.name
}))(NameDisplayer)

• You can also use setStore to test your not-so-dumb injected components with a mocked store.
• Keep your React tree clean. No more injected-MyComponent. The above example would look like this even though NameDisplayer is receiving name from store.user.name:

<App>
  <NameDisplayer />
</App>

• If this ends up in a npm package, you avoid the need to import your store locally, which sometimes is a mess when you are in a deep nested folder and makes moving files around painful.

import inject from 'mobx-react-inject'
// vs
import store from '../../../../store'

I'm reopening the issue to discuss precisely that:

• Do you think this is worth its own npm package? And if so:
  • Does it make sense to include it in this package?
  • Or it's best to create a new package, like for example mobx-react-inject?

cc: @mweststrate

from mobx-react-lite.

Maybe someone come in handy this typescript version of the HOC.

Utility function:

import { useObserver } from "mobx-react-lite";
import * as React from "react";

export function createPlug<Store>(context: React.Context<Store>) {
  return <OwnProps, SelectorOutput>(
    selector: (store: Store, ownProps: OwnProps) => SelectorOutput
  ) => {
    return (Component: React.SFC<OwnProps & SelectorOutput>) => {
      const displayName = Component.displayName || Component.name || "Untitled";
      Component.displayName = displayName;
      const HOC: React.SFC<OwnProps> = ownProps => {
        return useObserver(() => {
          return Component({
            ...ownProps,
            ...selector(React.useContext(context), ownProps)
          });
        });
      };
      HOC.displayName = `hoc(${displayName})`;
      return HOC;
    };
  };
}

Create plug function instance:

export const context = React.createContext(store);
export const plug = createPlug(context);

Usage example:

const ListItem = plug((store, ownProps: { id: string }) => ({
  openDoc: store.view.openDoc,
  isActive: store.view.docId === ownProps.id
}))(function ListItemComponent(props) {
  return (
    <li>
      <button disabled={props.isActive} onClick={() => props.openDoc(props.id)}>
        {props.id}
      </button>
    </li>
  );
});

from mobx-react-lite.

Using React.memo with a component that includes the useContext hook seems really dangerous.

From: https://reactjs.org/docs/react-api.html#reactmemo

If your function component renders the same result given the same props, you can wrap it in a call to React.memo for a performance boost in some cases by memoizing the result. This means that React will skip rendering the component, and reuse the last rendered result.

By including context inside a functional component wrapped with React.memo, you're implying that there's a very good chance that the component will not render the same result given the same props.

from mobx-react-lite.

Ok, then it seems great as it is. Even better than the original inject as it doesn't create a new component in your tree :)

from mobx-react-lite.

Well, as I said, I've never tried that example, so I guess it's good thing you brought it up. How are you using such HOC? Note that it requires selector as a first argument and component in second, so it should be something like following, but mostly shooting blinds here as I am not convinced it's a good thing to do in general.

const userSelector = createSelector(({ user }) => ({
    name: user.name,
    age: user.age
}))

function UiComponent({ name, age }) {
    return (
        <div>
            <div>Name: {name}</div>
            <div>Age: {age}</div>
        </div>
    )
}

export default inject(userSelector, UiComponent)

from mobx-react-lite.

Thanks for the reply.

Yeah that's essentially what I was trying, seems that it's a no-go. I've read the discussion on a useInject hook, does this discussion capture your thoughts on the subject? Just using the useContext hook as required in components is a preferred approach?

from mobx-react-lite.

I am not sure why it wouldn't be working. If you would prepare a reproduction repo or better a sandbox I would look at it. You can use my template codesandbox for an easy start.

from mobx-react-lite.

Sure thing. Here you go

from mobx-react-lite.

Alright, it's working ... https://codesandbox.io/s/rlv06114op

There were a few mistakes.

• Do not call it useInject as it's not a hook (not that it would be causing any error)
• The HOC always have to return a wrapped component, not a result of rendering
• You need to actually render injected component, not return it

I would appreciate if you could fix the example in the README and send in the PR.

from mobx-react-lite.

Got it. Thanks a lot for your time and the explanation.

from mobx-react-lite.

I've created a slightly more advanced inject with observer and ownProps, just like the original (mapperFunction: (stores, ownProps) => additionalProps).

https://codesandbox.io/s/qx18xrrvmq

const inject = (selector, baseComponent) =>
  React.memo(
    observer(ownProps => {
      const store = React.useContext(storeContext);
      const newProps = selector({ store, ownProps });
      return baseComponent(newProps);
    })
  );

from mobx-react-lite.

@luisherranz Not that I would use that personally, but you can improve it a more with useObserver instead. That way the observer won't be visible in debugger tree. One less nesting is always good :)

const inject = (selector, baseComponent) =>
  React.memo(
    (ownProps => {
      const store = React.useContext(storeContext);
      return useObserver(() => baseComponent(selector({ store, ownProps })));
    })
  );

from mobx-react-lite.

Codesandbox modified 👍

https://codesandbox.io/s/qx18xrrvmq

It would be perfect if we could change the component name. Right now is Memo:

from mobx-react-lite.

@luisherranz Nothing changes in that matter, you can just attach displayName on the component AFAIK.

from mobx-react-lite.

By including context inside a functional component wrapped with React.memo, you're implying that there's a very good chance that the component will not render the same result given the same props.

What's your suggestion then?

Nothing changes in that matter, you can just attach displayName on the component AFAIK.

How do you get the original component name?

from mobx-react-lite.

@luisherranz This should work just fine:

const inject = (selector, baseComponent) =>
    (ownProps) => {
        const store = React.useContext(storeContext);
        return useObserver(() => baseComponent(selector({ store, ownProps })));
    }

const MyInjectedComponent = inject(
    (store, ownProps) => {foo: store.computedFoo, ...ownProps},
    MyComponent
);

You certainly can include React.memo, just make sure it only wraps the baseComponent. That way the properties are already created. You really only want to pass f(x) = y (i.e. pure functions) to React.memo, because if f(x) = z in some cases, then React.memo would still only return y if it was computed first for a given x.

from mobx-react-lite.

How do you get the original component name?

Um, baseComponent.displayName || baseComponent.name ? :) Basics of creating HOC haven't changed with hooks or mobx in a bit :)

from mobx-react-lite.

Ok, this seems to work fine:

const inject = (selector, baseComponent) => {
  const component = ownProps => {
    const store = useContext(storeContext);
    return useObserver(() => baseComponent(selector({ store, ownProps })));
  };
  component.displayName = baseComponent.name;
  return component;
};

https://codesandbox.io/s/qx18xrrvmq

Any idea how to add React.memo to the baseComponent? Is it really needed or is it better to avoid it?

I thought the original inject implementation worked like that but I don't see it anymore. Maybe I was wrong.

from mobx-react-lite.

@luisherranz Memo is just an optimization so unless you feel like it's slowing down your app, I wouldn't be using it. As @larrymyers noted, you can end up with unexpected result of something not rerendering when you expect it.

https://twitter.com/danielk_cz/status/1064998451878998024

from mobx-react-lite.

Well, I am not a big fan of scoped variables that are essentially global. React context gives a great benefit that any part of the tree can override any parent level context value, but I guess it's not that essential in this case. Although, it should definitely warn if you try to call setStore multiple times.

Another big reason why I would not use this is TypeScript. It was a huge pain to get a correct typing with original inject and it's going to be mildly annoying with this as well because a scoped variable is untyped and for every use of inject you have to provide the type all over again.

For this reason, mainly, I don't think it fits into this package as we are full TypeScript here and everything needs to have good type safety. Feel free to make your own package.

from mobx-react-lite.

I guess it's not that essential in this case

Yes, React context doesn't make much sense here. It could be used though.

Another big reason why I would not use this is TypeScript.

We do not use TypeScript so I can't provide a typesafe implementation. I guess we'll have to use our own package then.

from mobx-react-lite.

Honestly, I don't think that many people would use this, HOC is slowly going to be replaced by hooks in my opinion. I would recommend you approach I recently documented in the article. It's a super easy way to have such small package in the team without polluting NPM with it. And if more people come asking for this, you can always publish it later.

from mobx-react-lite.

Also I've implemented typescript type to extract props type which will be injected to component. It can be useful when you want to split and use hoc and component separately. It's better to look at an example:

const HOC = plug((store, ownProps: { foo: number }) => ({
  items: store.items
}));

type HOCProps = PluggedProps<typeof HOC>; // -> { items: some[], foo: number };

const Component: React.SFC<HOCProps> = props => {
  return items.map(item => <div key={item.id}>{item.title}</div>);
};

// ...
// At the same file where `createPlug` placed
export type PluggedProps<
  T extends ReturnType<ReturnType<typeof createPlug>>
> = ComponentProps<PropsArg<T>>;

type PropsArg<T> = T extends (arg1: infer U, ...args: any[]) => any ? U : any;
type ComponentProps<T> = T extends React.SFC<infer X> ? X : null;

I imagine it would be cool to have it built in to mobx-react-lite. @FredyC what do you think?

from mobx-react-lite.

@geakstr Well, since the package does not export any sort of inject (I like your plug, btw), I don't see how it would fit here. The package is not meant like "garbage heap" really 😉

from mobx-react-lite.

@FredyC I named plug not inject because it does observer (your useObserver actually) internally, so inject may introduce confusion with inject from mobx-react. I didn't get your "garbage heap" statement. mobx-react-lite doesn't provide any analogue of inject from mobx-react. Functions from mobx-react-lite are great, but there is no way to pick pieces of state and inject them to component, without injecting store entirely.

from mobx-react-lite.

mobx-react-lite doesn't provide any analogue of inject from mobx-react

That's exactly right :) Have you read the discussion in #6? Also readme has a basic reasoning about it.

from mobx-react-lite.

Besides, the work on mobx-react V6 has started and there will be Provider/inject updated to use Context API. You might as well contribute your typing there.

from mobx-react-lite.

Well, you are kinda breaking the Rules of Hooks there by running useContext in the loop. In this case, it's not such a problem because it's a stateless hook (only reading something), but I would advise avoiding that just for your own sake.

Honestly, I don't follow why do you think that accessing by a string value is better (typo prone), but it's your choice. How often you need several stores within a single component?

// might be slightly more verbose, but it's clear what's happening there
const dataStore = useContext(stores.dataStore)

And I would personally, do this so you don't need to specify that array for each useStores call.

const useStores = (...selectors) => {
  ...
}
const { dataStore } = useStores("data");

Lastly, why not to wrap all stores into a single Context? I can imagine it can get pretty ugly if you have like 10 stores and each needs its own Provider in the tree.

from mobx-react-lite.

@FredyC you are right, it's a source of bugs, I'm deleting my comment to prevent any one to use it. But in my case, I usually need to import more than one store in one component, so importing every store I need is tedious. Is not that I like to use strings, but just using the inject hoc approach in mobx-react.

I will rewrite the hook to not break the hooks rules :)

from mobx-react-lite.