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 marcominetti/node-memwatch#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.

leakage/test/integration.test.js

Lines 11 to 18 in 5ee8c95

	it('throws an error when testing leaky code', () => {
	const objects = []
	expect(() => iterate(() => {
	const newObject = { foo: 'bar' }
	objects.push(newObject) // <= leak
	})).to.throw(MemoryLeakError, /Heap grew on \d subsequent garbage collections[\s\S]*Iterations between GCs: 30[\s\S]*Final GC details:/)
	})

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

leakage/lib/index.js

Lines 39 to 51 in 5ee8c95

	const runAndHeapDiff = () => {
	memwatch.gc()
	memwatch.gc()
	const heapDiff = new memwatch.HeapDiff()
	for (let index = 0; index < iterations; index++) {
	const result = iteratorFn()
	if (result && typeof result.then === 'function') {
	throw new Error(`Tried to use iterate() on an async function. Use iterate.async() instead.`)
	}
	}
	return heapDiff.end()
	}

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.