bug(SSR): memory leak when using renderToString

[eranhirs]

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
I am rendering my redux app on the server using the renderToString method.
I stress-loaded the system with ApacheBench and the memory consistently grows over time.

After some profiling and reading this code, it seems firebase/app.js creates a lot of observers at initialization.

I've been using react-redux-firebase for a while.
Would love to know if someone run into something similar and how can I cancel this behavior.

[prescottprue]

@eranhirs Really interesting use case (the stress loading sounds cool too).

Do you have a project you can share where you are doing this? Would love to replicate the issue so we can look into debugging.

[eranhirs]

It seems the issue starts when I initialize reactReduxFirebase on every request, it calls initializeApp and this code is creating event listeners.

I don't have a project I can share, but I based my implementation on this tutorial.
There is a link to a github project.

[eranhirs]

Initializing reactReduxFirebase once did the trick, outside of the configureStore method.
Thanks!

[eranhirs]

So this wasn't the only problem.
It seems componentWillUnmount is not called when calling renderToString .

I still see a lot of observers stacking up in an array attached to firebase auth.
Any idea where are these observers created?
I see firebaseConnect has a method watchEvents.

[bastianwegge]

Hey @eranhirs 👋,
do you maybe have the time to contribute a reproduction of your issue? I'd like to investigate but I can't find what you found without proper introduction to the code you're using. All the best!

[eranhirs]

I understand, I suggest we close this for now and re-open once there is a reproduction of the problem.

[bastianwegge]

You're pointing to a kind of serious error and I'm willing to help. Closing the Issue won't help. Do I understand correctly that this still occurs?

[eranhirs]

Since updating react-redux-firebase to version 2 it seemed to improve.
I need to re-run the stress tests and profile my code to see if what I wrote earlier is still relevant.
Currently, we can close this.

[prescottprue]

@eranhirs really great to hear, thanks for letting us know. @bastianwegge thanks for the help looking into it!

Going to close for now since it seems to have improved, but as mentioned we can always open again if there are steps to reproduce the issue.