Check extensions on a directory before redirecting

[cag]

I decided to change the behavior on the send module in a project and discovered that I really wanted sendFile to send things.json for a GET request to things when stuff like things/1.json exist.

[blakeembrey]

Stat will be undefined if there's an err.

[cag]

That's why I prefixed it with stat &&. That's idiomatic, right?

[dougwilson]

Hi! Thanks for your pull request, but it does not include any tests for the change you made, does not include documentation, and even breaks existing tests.

According to the tests this change broke, this change is actually breaking a feature of this module. Can you explain more what you change is doing, how it compares to the current functionality, and why it breaking our existing tests?

[cag]

I changed the tests to reflect what I mean and fixed an issue.

[cag]

According to the CI, this is only failing on node 0.8 now due to some package.json parsing thing. I've written a test, modified a test, and made sure all the existing tests pass. Could you please consider this PR for the next release?

[dougwilson]

Hi @cag! It looks like you never actually answered all my questions in #107 (comment) . So, since I didn't get an answer, I can only make assumptions on what you are trying to do based on the code changes.

This is a breaking change and will have to wait for the next major version at some point. Currently directories are preferred over extension-less files, and changing this will not be a backwards-compatible change.

[cag]

Sorry, I was a bit headstrong and forgot to elaborate. You're right that this change may potentially break distributions that depend on this package, but my feeling is that the behavior change is the natural one.

I changed the behavior of the extensions option so that for this:

resource.json
resource/
    item1.json
    item2.json

with extensions=['json'], resource, resource/item1, and resource/item2 returns the JSON files. I did it because OS's don't usually allow files to share names with directories, and because I wanted to stand a mock back-end up with serveStatic and connect.

Currently, send sends a redirect for resource with extensions set where a file exists with that extension. This change checks through the file extensions first before sending the redirect.

I changed the description for one of your tests to reflect the above, but I have fixed my code changes so that they don't break your tests.

[dougwilson]

Thanks, @cag! Yes, this change makes sense. The reason I bring up the it being a breaking change is because this conflict was known about when the extensions feature was added and it was decided at the time to prefer the directory over looking for a file with the given extension. Because of this history, I see your PR here as a proposal to change that behavior, not a bug fix. I hope that makes sense.

[cag]

That's totally fine. I didn't think of it as a bugfix. This is just the scenario I personally ran into in the wild.