Skip to content
This repository has been archived by the owner on Sep 10, 2022. It is now read-only.

memory leak of mapPropsStream at server side #328

Open
istarkov opened this issue Mar 7, 2017 · 6 comments
Open

memory leak of mapPropsStream at server side #328

istarkov opened this issue Mar 7, 2017 · 6 comments
Assignees

Comments

@istarkov
Copy link
Contributor

istarkov commented Mar 7, 2017

Seems like current mapPropsStream causes a memory leak at the server (server side rendering), and having that the idea to call subscribe at componentDidMount is not the thing what I want in most cases,
the workaround I see is to use at server side observable config like

const config = {
  fromESObservable: Rx.Observable.from,
  toESObservable: stream => stream.take(1) // take(1) is a solution
}

As an example how take will help https://jsfiddle.net/2j2esxyL/7/

So having that there will be no call of componentWillUnmount at server side,
take(1) will close the stream so there will be no memory leaks and all event listeners will be removed.

Even the solution above will not solve all the cases,
(the case at which mapPropsStream will wait something inside is not covered)
It seems like it will work for all cases I have internally.

I think if it's true (I haven't checked problem above) we need to add this into documentation.

@wuct ^

@wuct
Copy link
Contributor

wuct commented Mar 8, 2017

Found a related issue facebook/react#3714

@wuct
Copy link
Contributor

wuct commented Mar 8, 2017

It seems there is not to much we can do without detecting the runtime environment.

mapPropsStream will wait something inside

I am not sure what do you mean. Can you share an example?

@wuct
Copy link
Contributor

wuct commented Mar 8, 2017

Maybe we can make two subscriptions, one in componentWillMount and one in componentDidMount. The one in componentWillMont will unsubscribe immediately after setState.

    componentWillMount() {
      const subscription = this.vdom$.subscribe({
        next: vdom => {
          this.setState({ vdom }, subscription.unsubscribe)
        }
      })
      this.propsEmitter.emit(this.props)
    }

    componentDidMount() {
      this.subscription = this.vdom$.subscribe({
        next: vdom => {
          this.setState({ vdom })
        }
      })
    }

The one in componentDidMount just works like how it should work and will be unsubscribed in componentWillUnmount.

@istarkov
Copy link
Contributor Author

istarkov commented Mar 8, 2017

Immediate unsubscription will cause cancellation of user created streams, in all cases its not an expected behavior.
Will write an example of situation at which take(1) will not solve a problem later, now on mobile.

@avocadowastaken
Copy link
Contributor

I guess stream.take(1) is only good solution for SSR now. We need serverDidRender hook (can't find link to discussion).

@wuct
Copy link
Contributor

wuct commented Mar 8, 2017

Agree. I can't come up with other solutions.

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

No branches or pull requests

3 participants