Nodegit: Need to call `walker.start()` for `tree.walk()` not documented except in example

Created on 20 Nov 2018 · 2Comments · Source: nodegit/nodegit

System information

node version: v10.13.0
npm or yarn version: 6.4.1
OS/version/architecture: gLinux 64-bit (Google-internal, like Debian Testing)
Applicable nodegit version: v0.23.0 built from source

https://www.nodegit.org/api/tree/ has an example of how to use tree.walk(), but it doesn't mention a critical detail, which is mentioned in an example:
https://github.com/nodegit/nodegit/blob/1f84d31da984cb2c105bf669393438081eede8a9/examples/walk-tree.js#L22-L23

It looks like the start() method is added to the instance here:
https://github.com/nodegit/nodegit/blob/1f84d31da984cb2c105bf669393438081eede8a9/lib/tree.js#L145-L146

It took me quite a while to work out what the problem was, looking at the EventEmitter documentation would not have helped. I'm not sure if subclassing from EventEmitter is a good idea, but having a TreeWalker class where the this is documented would have made it easier to discover.

Good First Issue Help Wanted

Source

foolip

👍1

Most helpful comment

I ran into this as well. I want to add a couple of specific points:

The doc comment claims that walk returns an EventEmitter but the returned object actually has an extra method (start) that EventEmitters don't have.
Similarly, the TypeScript type definition file indicates that walk returns an EventEmitter so you get a compiler error when attempting to call start.

Here are specific suggestions for improvement:

In the walk documentation, tell people to call start.
Create a new type called TreeWalkerEventEmitter which is just like EventEmitter but it has an additional start method. Include this type in the nodegit documentation and add this type to the TypeScript type definition file.

rigdern on 1 Dec 2018

👍2

All 2 comments

I ran into this as well. I want to add a couple of specific points:

The doc comment claims that walk returns an EventEmitter but the returned object actually has an extra method (start) that EventEmitters don't have.
Similarly, the TypeScript type definition file indicates that walk returns an EventEmitter so you get a compiler error when attempting to call start.

Here are specific suggestions for improvement:

In the walk documentation, tell people to call start.
Create a new type called TreeWalkerEventEmitter which is just like EventEmitter but it has an additional start method. Include this type in the nodegit documentation and add this type to the TypeScript type definition file.