Document best practices: console.log #18

Closed
mjhea0 opened this Issue Jan 11, 2017 · 9 comments

Projects

None yet

2 participants

@mjhea0
mjhea0 commented Jan 11, 2017 edited

Thoughts on adding a few best practices around garbage collection. Adding a console.log for instance...

describe.only('test()', () => {
  it('does not leak when doing stuff', () => {
    iterate(6, () => { const test = new Person(); console.log(test); });
  });
});

function Person() {
  this.age = 1;
  this.name = 'mike';
}

vs

describe.only('test()', () => {
  it('does not leak when doing stuff', () => {
    iterate(6, () => { const test = new Person();  });
  });
});

function Person() {
  this.age = 1;
  this.name = 'mike';
}
@andywer andywer changed the title from console.log to Document best practices: console.log Jan 11, 2017
@andywer
Owner
andywer commented Jan 11, 2017

Hey @mjhea0.

Adding "best practices" to the documentation is a good point!
Concerning your example above... I haven't yet tried the code, but the console.log(test) should not introduce a leak or does it?

@mjhea0
mjhea0 commented Jan 11, 2017 edited

it prevents garbage collection. i don't think you can consider it a leak, but it does throw an error

@andywer
Owner
andywer commented Jan 11, 2017

I don't quite get why, though, to be honest. Why should console.log() keep a reference to the object after printing it? 🤔

But good to know. Thanks!
Any other best practice proposals? 😉

@mjhea0
mjhea0 commented Jan 11, 2017

http://stackoverflow.com/a/28859493/1799408

I imagine the same thing happens in node since it uses v8.

@andywer
Owner
andywer commented Jan 11, 2017 edited

Thanks for the link!

Even an explicit delete test after console.log(test) does not seem to help... (My bad. delete does not mark the object for GC in JS at all; it's really just for removing props from an object)

@mjhea0
mjhea0 commented Jan 11, 2017

firefox does the same. must be v8 and spidermonkey just keeping a reference for debugging purposes. another reason to git rid of those nasty console.logs!

@mjhea0
mjhea0 commented Jan 11, 2017

Probably a good idea to add in the docs that leakage tests should be kept separate from your normal test suite. They took like triple amount of the time to run!

@andywer
Owner
andywer commented Jan 11, 2017

Good point! Got to open another issue and make a list...

@andywer
Owner
andywer commented Jan 11, 2017

Closing this as there is now #19. Let's collect ideas there :)

@andywer andywer closed this Jan 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment