Wasted renders when using the <Observer> component?

[carlosagsmendes]

Hello,

I have a simple functional component where I'm using the <Observer/> component:

import React from "react";
import { Observer } from "mobx-react";

const MobxFunctionalComponentObserver = props => (
  <Observer
    inject={stores => ({ title: props.title, color: stores.store.color })}
    render={props => (
      <div>
        {props.title} {Date.now()}
        &nbsp;{props.color}
      </div>
    )}
  />
);

MobxFunctionalComponentObserver.displayName = "MobxFunctionalComponentObserver";
export default MobxFunctionalComponentObserver;

This component is always being re-rendered.

Here is a working example on codesandbox

In the same link I've created a similar stateless component that doesn't get re-rendered every single time:

import React from "react";
import { trace } from "mobx";
import { observer, inject } from "mobx-react";

const MobxFunctionalComponent = inject("store")(
  observer(({ store, title }) => (
    <div>
      {title} {Date.now()}
      &nbsp;{store.color}
    </div>
  ))
);

MobxFunctionalComponent.displayName = "MobxFunctionalComponent";
export default MobxFunctionalComponent;

Is this a bug or I'm I missing something?

Thanks in advance

[urugator]

I think it's a bug, because implementation seems completely wrong to me.
First of all it creates new InjectComponent during every render, which breaks react's reconciliation.
Secondly it applies inject prior to observer therefore the passed render actually doesn't run as part of observer (therefore isn't reactive etc).

[carlosagsmendes]

Thank you @urugator

[danielkcz]

Well, I think that a whole thing should be refactored into Observer being regular component (with render/children prop) in a first place and then turning it into HoC. It's kinda messy now and hard to track an actual issue. I might be able get to it, but no promises. If someone has more free time, feel free to do it.

/cc @Sunshine168

[mweststrate]

Correct @urugator , this is indeed a bug, sorry for not noticing it earlier :) PR's are welcome (including tests). Otherwise I'll take a stab at it next week or so

[Sunshine168]

@FredyC eemm, it is regretful for marking this future incorrectly.
Refactoring it to HOC look like this?

const comp = Observer(inject)(component)

i so busy during this period （orz） ,but if i can do this correctly , i will try

[duro]

This has me dead in the water with my React Native project, since use of FlatList requires the use of the Observer component.

[urugator]

@duro Observer is fine, just don't use the inject attribute

[mweststrate]

inject on Observer will be removed in the next version, it is just to fundamentally broken right now. Inject the hosting component instead, and access the injected stuff from the closure.

[danielkcz]

@mweststrate Well, I would love to see some example what do you mean. Sure, the inject is redundant and can be easily replaced with React Context API, but I cannot imagine what is the alternative to Observer?

[mweststrate]

Sorry for the confusion, Observer will stay, it just won't support that `inject` property anymore in the future Op di 24 apr. 2018 om 09:26 schreef Daniel K. <notifications@github.com>:

…

@mweststrate <https://github.com/mweststrate> Well, I would love to see some example what do you mean. Sure, the inject is redundant and can be easily replaced with React Context API <#448>, but I cannot imagine what is the alternative to Observer? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#423 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABvGhPJmVgY2khazyTPLL6wYtcIToX0Vks5trtOKgaJpZM4SGHuE> .