cannot install on node 10

[alvis]

memwatch-next, an upstream dependence, doesn't support v8 6.6 and therefore preventing leakage to be installed on node 10. See https://github.com/marcominetti/node-memwatch/issues/39

Should we swap memwatch-next with node-memwatch to solve the problem?

[andywer]

Hey @alvis. Thanks for pointing out!

Yeah, let‘s use the fork‘s fork. Seems like the easiest solution :)

Would you mind opening a PR?

[alvis]

happy to do so

[alvis]

okay. seems like it's not as straightforward as I thought. It's almost okay, but I got this test error if I swap memwatch-next with node-memwatch (also @airbnb/node-memwatch).

1) leakage throws an error when testing leaky code:
     AssertionError: expected [Function] to throw an error
      at Context.it (test/integration.test.js:17:17)

The indicates that the following code does not throw. https://github.com/andywer/leakage/blob/5ee8c955240997b2979e039c75db0e93cdc418e0/test/integration.test.js#L11-L18

I'm not an expert in this area, but it seems like to me that the problem is due to the change in V8's GC. i.e. the gc calls in the following lines may not work as intended https://github.com/andywer/leakage/blob/5ee8c955240997b2979e039c75db0e93cdc418e0/lib/index.js#L39-L51

any thought?

[andywer]

No idea yet 😕

Gotta read up on the V8 garbage collector changes. Maybe it has become harder to trigger a full GC 🤔

[alvis]

It turns out that a simple replacement is just fine. What's not fine is the pressure of the test. The default 30 iterations on a small object { foo: 'bar' } is too subtle to be detected. The test ended up passed when I increase the number of iterations to 300. But in the real world scenario, 30 is fine I think as we'll be testing object at least 10x bigger than { foo: 'bar' }.

This issue can be closed once #27 is merged.

[andywer]

Closed by #27. Published as v0.4.0 🚀

Thought about making it v1.0, but let's give it a little bit of time to be battle-tested first.