Skip to content

Conversation

@piranna
Copy link
Contributor

@piranna piranna commented Aug 13, 2016

No description provided.

@dougwilson
Copy link
Contributor

Thanks! Please add tests for the new feature and documentation :)

In addition, can you provide what the use-case for this new feature is? Perhaps a code example of what you can now do with the feature that was not possible before.

@piranna
Copy link
Contributor Author

piranna commented Aug 14, 2016

If you listen for the 'directory' event, this allow to know what is the directory that's being requested, otherwise is not possible to know.

@dougwilson
Copy link
Contributor

otherwise is not possible to know

how so? As far as I can tell, all you're doing is passing back whatever the user provided as the path argument to the send function originally, which of course the calling code would already know, because that's the same value it passed to this module in the first place.

If this is not the case, can you please provide code that shows what exactly you mean? As far as I can tell, this is not a useful addition, since it doesn't actually solve anything, only adding a second way to do something, which is more to maintain.

@piranna
Copy link
Contributor Author

piranna commented Aug 14, 2016

Of course you have the path before, but this way you have it as parameter instead to force to get it from the closure or some external variable, making this more flexible and easier to use. Events should have available all the info they need and don't depend on the context for each call, doing so they lost their meaning.

@dougwilson
Copy link
Contributor

Hi @piranna, but then what can you even do in the event listener? If you need to, for example, actually send a response, you'll need to use a closure to respond to the correct res object. Please provide code that shows what exactly you mean as far as I can tell, this is not a useful addition, since it doesn't actually solve anything, only adding a second way to do something, which is more to maintain.

@dougwilson
Copy link
Contributor

Clearly I did not handle this issue well, and for that I'm sorry. I am landing your PR with included tests and documentation. res is also added to the arguments to be similar to he headers event (and also because the expectation is to write a response in the event).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants