cannot install on node 10
alvis opened this issue · 6 comments
memwatch-next, an upstream dependence, doesn't support v8 6.6 and therefore preventing leakage to be installed on node 10. See marcominetti/node-memwatch#39
Should we swap memwatch-next with node-memwatch to solve the problem?
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?
happy to do so
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.
leakage/test/integration.test.js
Lines 11 to 18 in 5ee8c95
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
Lines 39 to 51 in 5ee8c95
any thought?
No idea yet 😕
Gotta read up on the V8 garbage collector changes. Maybe it has become harder to trigger a full GC 🤔
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.