Fix map memory leak

[brunorzn]

using object as keys in map can cause memory leaks since if the reference to the same-value object is different, it will not find it.

This is the case, for example, when you use InMemoryCache during SSR.

Example of code that reproduces the leak :

function loop() {
  console.log('Will loop in a few seconds...');
  return new Promise((resolve) => {
    setTimeout(() => {
      for (let i = 0; i < 10000; i += 1) {
        console.log('writing some data in memory cache...');
        const cache = new InMemoryCache();
        cache.writeData({ data: { Test: { value: 7, __typename: 'Test' } } });
        console.log('done.');
      }
      resolve();
    }, 3000);
  });
}

async function run() {
  while (true) {
    await loop();
  }
}

run();

also, see issue apollographql/apollo-link-state#248

Checklist:

• If this PR is a new feature, please reference an issue where a consensus about the design was reached (not necessary for small changes)
• Make sure all of the significant new logic is covered by tests
• If this was a change that affects the external API used in GitHunt-React, update GitHunt-React and post a link to the PR in the discussion.

[apollo-cla]

@brunorzn: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

[hwillson]

Hi @brunorzn - it looks like the tests need to be adjusted a bit, to accommodate for the cache removal. If you don't mind adjusting the tests, I'll do a final review when everything is passing. Thanks!

[brunorzn]

@hwillson : sorry about those failing tests. I did it too quickly during a meeting. I just felt like removing this couple of "should memoize and return cached results" tests.

Also, strangely, I had to modify the end of this test file, by adding an extra }), see commit 4557ca7 : what do you think ?

The tests should now pass.

[hwillson]

Awesome @brunorzn - this all looks great! Thanks very much! I've updated the CHANGELOG, and will merge once the tests have completed. Thanks again!

[hwillson]

Wow, looks like the describe on line 917 wasn't closed - good catch!

[brunorzn]

Yeah I had to like triple check this to be sure it wasn't me ;) (basically, eslint caught it, not me)

[brunorzn]

Thanks @hwillson ! Hopefully this will be merged soon !

[benjamn]

This change will cause some problems for #3394, since I have been relying on the reuse of query document objects across repeated cache reads, and this change causes the document to be different every time.

I think a better way to fix the memory leak would be to use a WeakMap for the cache instead of a Map. I'm also worried about the performance implications of all this extra document copying, so I would move to delay publishing this change until we can find a solution that preserves the caching (without memory leaks).