Skip to content

Changes in express files breaks livereload #42

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
johngouf opened this issue Dec 31, 2013 · 29 comments
Closed

Changes in express files breaks livereload #42

johngouf opened this issue Dec 31, 2013 · 29 comments

Comments

@johngouf
Copy link

Hi there!

first of all congrats for this wonderful generator!
I noticed the following. If I make any changes to files under lib ( for express ), it does reload the browser but I get:

Oops! Google Chrome could not connect to localhost:9000

If I refresh manually it works ok with the changes made.
I know its not a big issue but thought it could be useful to know.

Thanks a lot!

@DaftMonk
Copy link
Member

Thanks for pointing this out. For me, this only happens when I introduce a syntax error that trips up the server and breaks the livereload script. So as long as the changes you make don't cause any errors it should reload correctly.

That said, I think a better way of dealing with this would be to have jshint validate your server code before it reloads. This would prevent the server from crashing if an error was introduced but would let you know where to fix it.

I'll probably release an update to the generator that adds this, but it should be pretty simple to add to your project.

Create a .jshintrc file in the lib folder with the following options:

{
  "node": true,
  "esnext": true,
  "bitwise": true,
  "camelcase": true,
  "curly": true,
  "eqeqeq": true,
  "immed": true,
  "indent": 2,
  "latedef": true,
  "newcap": true,
  "noarg": true,
  "regexp": true,
  "undef": true,
  "strict": true,
  "smarttabs": true
}

Then in your gruntfile under the jshint task add an option for the server code:

 jshint: {
 ...
    server: {
      options: {
        jshintrc: 'lib/.jshintrc'
      },
      src: [ 'lib/{,*/}*.js']
    }
}

Finally, update the express task with the new jshint:server task.

express: {
  files: [
    'server.js',
    'lib/{,*//*}*.{js,json}'
  ],
  tasks: ['newer:jshint:server', 'express:dev'],
  options: {
    livereload: true,
    nospawn: true //Without this option specified express won't be reloaded
  }
}

I'm sure there will still be some that get through and crash the server, but that should catch most of the errors you might introduce.

Hope that helps!

@johngouf
Copy link
Author

johngouf commented Jan 1, 2014

Hi there, thanks for the prompt response!
Unfortunately, still when I change for instance in file /lib/controllers/api.js the line with:

name : 'HTML5 Boilerplate 2',

(without syntactical errors)
it produces the error I was writing about. Refreshes the browser but with displaying the error. If you refresh manually then it works ok.
Is it possible that is something wrong with my setup? On your end it works ok this scenario?

@DaftMonk
Copy link
Member

DaftMonk commented Jan 2, 2014

Hmm, that does actually work for me. So it probably is something wrong with your setup, but I'm not sure where that would be going wrong.

@DaftMonk DaftMonk reopened this Jan 2, 2014
@johngouf
Copy link
Author

johngouf commented Jan 2, 2014

I really can't figure out what the problem can be. I was using nodejs v.0.10.24 from nvm,
I changed this, and used the one installed from ppa for ubuntu but same issue occurs.
Everything goes smoothly both in the installation and in the generation of the project.
Could I try to check something else from my side? :(

@DaftMonk
Copy link
Member

DaftMonk commented Jan 2, 2014

Well the way you describe it, the server restarts after you make a change, but it takes you to a 404 page.

What happening behind the scenes is that there's an express watch task that monitors changes in your file. When it detects a change, it restarts the server. When its finished restarting the server it triggers a livereload of your browser.

Strangely, it sounds like all of that is working for you, but that the livereload is happening before the server has finished restarting.

Maybe you could delay the livereload task for a second or two till you're sure the server has finished restarting.

In your gruntfile, try adding this to the express watch task options:

options: {
  debounceDelay: 1000,
  livereload: true,
  nospawn: true //Without this option specified express won't be reloaded
}

I don't know if debounceDelay works the way I think it does. If that doesn't work, you might want to check out the grunt-contrib-watch docs and see if theres anything else that could do the trick.

@johngouf
Copy link
Author

johngouf commented Jan 2, 2014

Unfortunately even that option didn't do the trick. I checked a lot in the contrib-watch docs without finding anything. Also I increased a lot the debounceDelay without having any effect.
Maybe its another task which triggers the reload of the browser?
Also what node version are you using?

@DaftMonk
Copy link
Member

DaftMonk commented Jan 2, 2014

It's definitely the express watch task which triggers the reload of the browser.

My version of node is 0.10.16, but that probably won't do you much good. I wish I could be of more help...

@johngouf
Copy link
Author

johngouf commented Jan 2, 2014

Used the same node version, same behaviour.
Anyway, I will keep digging to see what is wrong with my setup and will let you know if I discover anything

@ami7ava
Copy link

ami7ava commented Jan 3, 2014

I am also facing the similar problem. For now I have disabled livereload for express task. My node version is 0.10.23.

@jeef3
Copy link
Contributor

jeef3 commented Jan 5, 2014

When I split out the express and livereload tasks I was worried about the implications of reloading the browser just because the server has changed. In some cases this is desirable, but sometimes it is not.

As @abasak mentioned, you can change this behaviour by disabling LiveReload in the express task:

express: {
  files: [
    'server.js',
    'lib/{,*//*}*.{js,json}'
  ],
  tasks: ['newer:jshint:server', 'express:dev'],
  options: {
    livereload: true,
    nospawn: true //Without this option specified express won't be reloaded
  }
}

becomes:

express: {
  files: [
    'server.js',
    'lib/{,*//*}*.{js,json}'
  ],
  tasks: ['newer:jshint:server', 'express:dev'],
  options: {
    // livereload removed, or set to false
    nospawn: true //Without this option specified express won't be reloaded
  }
}

@kjellski
Copy link

I'm having the same issues here. If I save the server.js file, even without changes, it starts reloading, but stops immediately after the database population step. Then grunt completely hangs.

@DaftMonk
Copy link
Member

@kjellski Really? Maybe we should consider switching from grunt-express-server to nodemon https://github.com/ChrisWren/grunt-nodemon.

Need someone to see if it fixes the problem, but it would be worth it to try.

@kjellski
Copy link

@DaftMonk unfortunately I'm pretty bad at using and configuring grunt, I've dont nothing but copy paste parts of configs together honestly. But I would like to help, can you tell me what I should do to test this? Would I just hang that task in default and see whether it works or not? :/ sorry, I've got really no clue here...

@DaftMonk
Copy link
Member

Can't test it right now, but I think this is what you'll need to do:

  1. npm install -g nodemon
  2. npm install --save-dev grunt-nodemon
  3. Add the nodemon configuration to the initConfig object
nodemon: {
  dev: {
    options: {
      file: 'server.js',
      nodeArgs: ['--debug'],
      watchedExtensions: ['js'],
      watchedFolders: ['lib'],
      env: {
        PORT: process.env.PORT || 9000
      }
    }
  }
},
  1. Remove the 'express:dev' task from the express watch configuration
  2. In the serve task replace 'express:dev' with 'nodemon:dev'

edit: got a chance to run this, i thought nodemon triggered a livereload but apparently it doesn't. I'm starting to rethink this whole idea. I'm going to try to tinkering with an idea for how we could fix this.

@DaftMonk
Copy link
Member

Ok heres another idea. I hate shooting in the dark but I gotta try it.

Try seperating the express livereload into its own watch task, since they run consecutively.

  express: {
    files: [
      'server.js',
      'lib/**/*.{js,json}'
    ],
    tasks: ['newer:jshint:server', 'express:dev'],
    options: {
//      livereload: true,
      nospawn: true //Without this option specified express won't be reloaded
    }
  },
  expressReload: {
    files: [
      'server.js',
      'lib/**/*.{js,json}'
    ],
    options: {
      livereload: true
    }
  }

If that still doesn't work, you could try adding a delay task:

grunt.registerTask('5sec', 'Delay for express restart.', function() {
  var done = this.async();
  setTimeout(function() {
    done();
  }, 5000);
});

And adding that to the express watch task stack:

tasks: ['newer:jshint:server', 'express:dev', '5sec'],

It sucks that I can't replicate the issue, this would be a lot easier to test/debug if I could. If anyone wants to give that a try let me know how it goes :)

@jeef3
Copy link
Contributor

jeef3 commented Jan 13, 2014

I originally used grunt-nodemon but switched back to grunt-express-server because grunt-nodemon expected all your watch set-up to be in its configuration, rather than a watch task.

nodemon (and grunt-nodemon) does have much better error reporting though, like what line number has broken it if it fails to restart.

@DaftMonk
Copy link
Member

@jeef3 Interesting, I'll try it out.

Do you think it would it be possible to switch over to it without losing any of the watch functionality we have?

@jeef3
Copy link
Contributor

jeef3 commented Jan 13, 2014

It's possible, worth a shot. If I get a chance (and no one beats me to it) I'll give it a shot tonight.

@DaftMonk
Copy link
Member

It looks like we might be able to use grunt concurrent to run watch tasks alongside nodemon. I wonder if you can configure nodemon to open a new browser window when it starts as well.

@jeef3
Copy link
Contributor

jeef3 commented Jan 13, 2014

That's actually the other problem I ran into. I struggled to get it working properly with concurrent. Just couldn't get open to run.

@DaftMonk
Copy link
Member

Ah, yeah. It would be nice if you could just tell nodemon to run the open task when its finished starting up, but I doubt it's that simple.

@diiimo1973
Copy link

We had the same problem.
Our experience: The reload is triggered before the server is back.
Our solution: A waiting task.

// task that simply waits for 1 second, usefull for livereload
grunt.registerTask('wait', function () {
    grunt.log.ok('Waiting...');

    var done = this.async();

    setTimeout(function () {
        grunt.log.writeln('Done waiting!');
        done();
    }, 2000);
});

in watch task:

        express: {
            files: [
                'server.js',
                'server/**/*.{js,json}'
            ],
            tasks: ['express:dev', 'wait'],
            options: {
                livereload: true,
                nospawn: true //Without this option specified express won't be reloaded
            }
        }

@DaftMonk
Copy link
Member

@diiimo2011 Thanks for posting that. What is the minimum timeout you need for that to continue to work?

Also it would be helpful if anyone else who's having this problem could try that and report back. I'd like to know how it worked so I could include that snippet in the generator.

@diiimo1973
Copy link

Here goes 1 second for Mac and Linux. Windows better 2 seconds. We have a team with Win, Linux and Mac and we use 2 seconds.

@kjellski
Copy link

If nobody else does this before, I'll try it this weekend, hope to have more time then. @DaftMonk I hope to get a look on your work regarding the model and service generation...

@jeef3
Copy link
Contributor

jeef3 commented Feb 14, 2014

@DaftMonk I figured this discussion belonged in here.

I'm sure it will be fine for most cases. I never looked into it, but did the delay option in grunt-express-server not help? Or even the output? A shame there's no event callbacks like there is in nodemon.

@DaftMonk
Copy link
Member

I could never get nodemon to behave exactly how I wanted it. It might be possible to get it working with a lot of tweaking, but for now, this is a simpler solution.

@jeef3
Copy link
Contributor

jeef3 commented Feb 14, 2014

Yeah, same, I had no luck with nodemon + LiveReload. Just... a shame is all 😄

Agreed.

@davisford
Copy link

I'm seeing the same issue. I have the wait task from @diiimo2011 in my Gruntfile

This is the wait task:

  // Used for delaying livereload until after server has restarted
  grunt.registerTask('wait', function () {
    grunt.log.ok('Waiting for server reload...');

    var done = this.async();

    setTimeout(function () {
      grunt.log.writeln('Done waiting!');
      done();
    }, 2000);
  });

...and the watch.express config

express: {
        files: [
          'server.js',
          'lib/**/*.{js,json}'
        ],
        tasks: ['newer:jshint:server', 'express:dev', 'wait'],
        options: {
          livereload: true,
          nospawn: true //Without this option specified express won't be reloaded
        }
      }

This is the relevant output on the console:

Running "express:dev" (express) task
Starting background Express server
debugger listening on port 5858

Running "open:server" (open) task

Running "watch" task
Waiting...
Express server listening on port 9000 in development mode
db connection open

I don't ever see the grunt.log.task('Waiting for server reload...') output.

The task is there, and will run if called -- seems to indicate watch:express isn't being run?

$grunt wait
Running "wait" task
>> Waiting for server reload...
Done waiting!

Done, without errors.


Execution Time (2014-05-05 06:02:52 UTC)
wait      2s  ▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇ 100%
Total 2s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants