Skip to content

res.write not called, empty returned response #43

Closed
wants to merge 2 commits into from

2 participants

@darvelo
darvelo commented May 3, 2013

When I try to use the livereloadSnippet middleware in my grunt-express app as here, I get an empty response on my routes.

Looking at the code, one comment says, before overwriting res.write:

  // Bypass write until end
  var inject = res.write = function (string, encoding) {

But the call to the original res.write doesn't seem to take place. If I cache the value and include it before a call to the original res.end, I get the response I'm looking for - the livereload snippet included at the end of the body.

@darvelo
darvelo commented May 3, 2013

Okay, I think I understand what's going on.

It seems that res.write is overwritten to prevent it from being used, in order to use res.end at the end. And this makes sense, because in the node.js docs, it says response.end, when called with a data argument, will be the same as res.write(data, encoding); res.end();. The issue here is that the code in res.end literally calls res.write, and if res.write is overwritten, it won't send any data. I'll see if I can come up with a small code change and reconfigure the tests now that I have them working.

@darvelo
darvelo commented May 3, 2013

Okay, that should do it.

  • No tests needed to be changed since I didn't change the behavior.
  • Travis build fails on Node 0.10 I think because grunt-simple-mocha should be specified for ~0.4.0 in package.json Not sure if you want to do that now so I didn't include it.
@shama
grunt member
shama commented May 3, 2013

Hey! Thanks for the pull request. Just wanted to catch you before you did any more work on this. Please check out issue #32. This task will be deprecated soon as livereload has moved to the watch task: https://github.com/gruntjs/grunt-contrib-watch#live-reloading

We recommend including the livereload.js script via a template mechanism or copying and pasting the script tag. As opposed to injecting with connect. But if you would like to inject with connect try: https://github.com/intesso/connect-livereload (untested 3rd party). Thanks!

@darvelo
darvelo commented May 4, 2013

Oh! Cool, the new method makes the flow easier to grok and use. Thanks! So then is this task no longer being updated, or taken off npm? Not sure if I should close the pull request at this stage.

@shama
grunt member
shama commented May 4, 2013

We just released the new watch task today. We'll likely npm deprecate this soon. So yes this task is no longer being updated. We appreciate the pull request though. Thanks!

@shama shama closed this May 4, 2013
@darvelo darvelo referenced this pull request in blai/grunt-express May 4, 2013
Closed

livereload snippet with grunt-express? #11

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.