SSR: store processed documents on the Link instance to avoid memory leak

[valerybugakov]

Prevents memory leak when apollo-link-rest is used on the server caused by storing processed documents in the module closure.

[codecov-io]

Codecov Report

Merging #209 into master will decrease coverage by 0.03%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #209      +/-   ##
==========================================
- Coverage   92.71%   92.68%   -0.04%     
==========================================
  Files           3        2       -1     
  Lines         412      410       -2     
  Branches      121      121              
==========================================
- Hits          382      380       -2     
  Misses         28       28              
  Partials        2        2

Impacted Files	Coverage Δ
src/restLink.ts	92.64% <100%> (+0.16%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 62ca581...62bc8e0. Read the comment docs.

[fbartho]

Should this be instead cached on the query instead of on the link? I wasn’t aware of this cache, I’m not exactly certain I understand what’s being cached and the risk.

[valerybugakov]

@fbartho it might be cached on the query too, what's the difference? I didn't want to change the logic, just wanted to fix the leak.

On the server, we recreate restLink for every request along with apollo-client etc. to avoid redundant caching between request which results in memory leaks.

const removed = new Map();

This cache cannot be cleared via recreating restLink because the module closure preserves it. That's why it grows with every query without any limits till server collapses with not enough memory error.

Let me know if it makes sense.

[valerybugakov]

Also, I checked the code in apollo-link-state, it seems that they removed cache completely. If we want to mimic this behavior (as noted here) that would solve SSR issue too.

[valerybugakov]

@fbartho any thoughts on this one?

[fbartho]

@valerybugakov I tagged this with a release milestone, to make it clear I hope to release a fix soon (I was on vacation when you submitted this).

Question for you: Would you mind researching why they removed their cache? My preference is to trust their performance analyses.

If they had a reason, then we can copy their code again here. Removing the cache would be simpler to maintain!

[fbartho]

@valerybugakov -- I'm working on a new release here, did you see my question re: "Why did they remove their cache?"

[valerybugakov]

Hey @fbartho, I didn't have time to do this research

[fbartho]

I'm going the get this out in a beta release today!

[fbartho]

Published under: https://github.com/apollographql/apollo-link-rest/releases/tag/v0.8.0-beta.0