linemanjs / lineman

Lineman helps you build fat-client JavaScript apps. It produces happiness by building assets, mocking servers, running specs on every file change
MIT License
1.18k stars 81 forks source link

Plugin for grunt-open? #185

Closed searls closed 9 years ago

searls commented 10 years ago

(@davemo brought this up in his codemash talk.)

It'd be nice to have a lineman-plugin that'd add to the dev lifecycle to add open & live reload.

jayharris commented 10 years ago

With grunt-contrib-watch supporting livereload, grunt-live-reload has been deprecated.

Since grunt-contrib-watch fully supports nospawn, could we drop grunt-watch-nospawn in favor of grunt-contrib-watch? If so, we could also default-enable livereload.

I have grunt-contrib-watch working under nospawn on a local spike. I know what triggered the fork a year ago, but I'm not familiar enough with the underlying reasons to know why (if at all) that lineman can't move back to grunt-contrib-watch, now.

Is this something we want to pursue? If so, I've got it staged and ready to commit.

jasonkarns commented 10 years ago

dude, you just poked the bear

jasonkarns commented 10 years ago

@jayharris :-)

We've attempted to move back to grunt-contrib-watch a couple times and have always been met with instability and weird quirks with directories not being watched correctly, tasks not re-running correctly and general mayhem.

I'm all for moving back, but your branch will probably need to 'bake' for a while as a separate prerelease before merging.

jayharris commented 10 years ago

As I say, it looks to me that grunt-contrib-watch was forked over a year ago because of the nospawn issue. But the fork was made during development of Watch v1.5, which eventually released as v2.0. It's on v0.5.3, now; a lot has changed, including the implementations for spawn : false and livereload : true.

If there is a technical reason to maintain the split, awesome; I'm completely cool with that (though I would like to know what they are, so I can be aware of them when reviewing future modifications to contrib-watch). But if not, it doesn't seem that hard to switch back, and we can pick up a few freebies along the way.

searls commented 10 years ago

Hey @jayharris, three thoughts. 

  1.  As Jason mentioned, we've tried to get off our fork at least twice (since they reintroduced nospawn) and each time we were met with problems. Initially, the problem was that at most one watch task could trigger for one file change, even if the file that changed would satisfy multiple globs. (I'm not sure that's ever been fixed, I never got it working).  The second problem was just a myriad of confusing, hard-to-reproduce failures to fire tasks upon certain file change events, especially around edges like "first file added to a directory" and "last file deleted from a directory."
  2. We've heard tell of much, much improved watch performance from @davemo at Shopify, but the dude continues to not loop us in on what and how they're doing. :-) I've heard really fantastic claims and I want to learn more and vet those out before we potentially change
  3. @airportyh wrote his own watched, I think called fireworm, which I also want to evaluate. Seems great at first blush but at the same time I can't get testem to see changes occur on windows so it may not work there. 

We're all a bit burnt out on this topic because the basic watch we have is "good enough" for small projects and every time we've tried to change we've run into obstacles.

On Tue, Jan 14, 2014 at 3:30 PM, Jay Harris notifications@github.com wrote:

As I say, it looks to me that grunt-contrib-watch was forked over a year ago because of the nospawn issue. But the fork was made during development of Watch v1.5, which eventually released as v2.0. It's on v0.5.3, now; a lot has changed, including the implementations for spawn : false and livereload : true.

If there is a technical reason to maintain the split, awesome; I'm completely cool with that (though I would like to know what they are, so I can be aware of them when reviewing future modifications to contrib-watch). But if not, it doesn't seem that hard to switch back, and we can pick up a few freebies along the way.

Reply to this email directly or view it on GitHub: https://github.com/testdouble/lineman/issues/185#issuecomment-32304198

davemo commented 10 years ago

Here's our experimental, directory watch only, which works amazingly fast :)

module.exports = function(grunt) {

  var _ = grunt.util._;
  var fs = require('fs');

  grunt.registerTask('watcher', 'experimental directory only', function() {
    var ETERNITY = 60000 * 60;
    var pathTaskMap = {};
    var watched = {};
    var folders = {};
    var intervalId;
    var subFolderToRootMap = {};
    var taskDone = this.async();

    var watchcfg = grunt.config('watcher');
    var targets  = Object.keys(watchcfg).filter(function(key) {
      return typeof watchcfg[key] !== 'string' && !Array.isArray(watchcfg[key]);
    });
    targets = targets.map(function(target) {
      // Fail if any required config properties have been omitted.
      target = ['watcher', target];
      this.requiresConfig(target.concat('files'), target.concat('tasks'));
      return grunt.config(target);
    }, this);

    var cachePathTaskMappings = function(targets) {
      _(targets).each(function(target) {
        target.tasks.push("emit:compile_end")
        _(target.files).chain().flatten().each(function(file) {
          pathTaskMap[file] = target.tasks;
        });
      });
    };

    cachePathTaskMappings(targets);
    grunt.log.writeln('Waiting...');
    var changes = {};
    var nameArgs = this.nameArgs;

    var patterns = _.pluck(targets, 'files');
    var getFolders = function() {
      return grunt.file.expand({filter: 'isDirectory', cwd: process.cwd()}, patterns);
    };
    var getFiles = function() {
      return grunt.file.expand({filter: 'isFile', cwd: process.cwd()}, patterns);
    };

    function walk(path) {
      var results = [];
      var list = fs.readdirSync(path);
      list.forEach(function(file) {
        file = path + '/' + file;
        if( !fs.existsSync(file) ) {
          return;
        }
        var stat = fs.statSync(file);
        if (stat && stat.isDirectory()) {
          results.push( file );
          results = results.concat(walk(file));
        }
      });
      return results;
    }

    function watch(path) {
      if(!watched[path]) {
        watched[path] = fs.watch(path, function(event, triggeringPath) {
          if( triggeringPath ) {
            if( triggeringPath.indexOf(".") == 0 || triggeringPath.indexOf("~") > 0 || triggeringPath.indexOf("#") == 0 ) {
              return;
            }
            if( !pathTaskMap[path] ) {
              path = subFolderToRootMap[path];
            }
          }
          changes[path] = true;
          handleChange(path, triggeringPath);
        });
      }
    }

    function unwatch(path) {
      if(watched[path]) {
        watched[path].close();
        delete watched[path];
      }
    }

    var handleChange = _.debounce(function (path, file) {
      if(file) {
        grunt.log.ok('changed', file);
      } else {
        grunt.log.ok('changed', path);
      }
      clearInterval(intervalId);
      Object.keys(watched).forEach(unwatch);
      grunt.log.ok();
      grunt.event.emit('compilationTasks', Object.keys(changes).length)
      Object.keys(changes).forEach(function(path) { grunt.task.run(pathTaskMap[path]).mark() });
      grunt.task.run(nameArgs);
      changes = {};
      taskDone();
    }, 250);

    function addWatches() {
      getFiles().forEach(function(path) { watch(path); });
      getFolders().forEach(function(path) {
        watch(path);
        subFolderToRootMap[path] = path;
        if (!folders[path]) {
          folders[path] = walk(path);
          folders[path].forEach(function(dir) { subFolderToRootMap[dir] = path; } );
        }
        folders[path].forEach( watch);
      });
    }

    addWatches();

    intervalId = setInterval( function() {
      addWatches();
    }, ETERNITY );
  });
};

Go nuts :)

searls commented 10 years ago

What does directory watch mean?

On Tue, Jan 14, 2014 at 3:41 PM, David Mosher notifications@github.com wrote:

Here's our experimental, directory watch only, which works amazingly fast :)

module.exports = function(grunt) {
  var _ = grunt.util._;
  var fs = require('fs');
  grunt.registerTask('watcher', 'experimental directory only', function() {
    var ETERNITY = 60000 * 60;
    var pathTaskMap = {};
    var watched = {};
    var folders = {};
    var intervalId;
    var subFolderToRootMap = {};
    var taskDone = this.async();
    var watchcfg = grunt.config('watcher');
    var targets  = Object.keys(watchcfg).filter(function(key) {
      return typeof watchcfg[key] !== 'string' && !Array.isArray(watchcfg[key]);
    });
    targets = targets.map(function(target) {
      // Fail if any required config properties have been omitted.
      target = ['watcher', target];
      this.requiresConfig(target.concat('files'), target.concat('tasks'));
      return grunt.config(target);
    }, this);
    var cachePathTaskMappings = function(targets) {
      _(targets).each(function(target) {
        target.tasks.push("emit:compile_end")
        _(target.files).chain().flatten().each(function(file) {
          pathTaskMap[file] = target.tasks;
        });
      });
    };
    cachePathTaskMappings(targets);
    grunt.log.writeln('Waiting...');
    var changes = {};
    var nameArgs = this.nameArgs;
    var patterns = _.pluck(targets, 'files');
    var getFolders = function() {
      return grunt.file.expand({filter: 'isDirectory', cwd: process.cwd()}, patterns);
    };
    var getFiles = function() {
      return grunt.file.expand({filter: 'isFile', cwd: process.cwd()}, patterns);
    };
    function walk(path) {
      var results = [];
      var list = fs.readdirSync(path);
      list.forEach(function(file) {
        file = path + '/' + file;
        if( !fs.existsSync(file) ) {
          return;
        }
        var stat = fs.statSync(file);
        if (stat && stat.isDirectory()) {
          results.push( file );
          results = results.concat(walk(file));
        }
      });
      return results;
    }
    function watch(path) {
      if(!watched[path]) {
        watched[path] = fs.watch(path, function(event, triggeringPath) {
          if( triggeringPath ) {
            if( triggeringPath.indexOf(".") == 0 || triggeringPath.indexOf("~") > 0 || triggeringPath.indexOf("#") == 0 ) {
              return;
            }
            if( !pathTaskMap[path] ) {
              path = subFolderToRootMap[path];
            }
          }
          changes[path] = true;
          handleChange(path, triggeringPath);
        });
      }
    }
    function unwatch(path) {
      if(watched[path]) {
        watched[path].close();
        delete watched[path];
      }
    }
    var handleChange = _.debounce(function (path, file) {
      if(file) {
        grunt.log.ok('changed', file);
      } else {
        grunt.log.ok('changed', path);
      }
      clearInterval(intervalId);
      Object.keys(watched).forEach(unwatch);
      grunt.log.ok();
      grunt.event.emit('compilationTasks', Object.keys(changes).length)
      Object.keys(changes).forEach(function(path) { grunt.task.run(pathTaskMap[path]).mark() });
      grunt.task.run(nameArgs);
      changes = {};
      taskDone();
    }, 250);
    function addWatches() {
      getFiles().forEach(function(path) { watch(path); });
      getFolders().forEach(function(path) {
        watch(path);
        subFolderToRootMap[path] = path;
        if (!folders[path]) {
          folders[path] = walk(path);
          folders[path].forEach(function(dir) { subFolderToRootMap[dir] = path; } );
        }
        folders[path].forEach( watch);
      });
    }
    addWatches();
    intervalId = setInterval( function() {
      addWatches();
    }, ETERNITY );
  });
};

Go nuts :)

Reply to this email directly or view it on GitHub: https://github.com/testdouble/lineman/issues/185#issuecomment-32305104

davemo commented 10 years ago

Here's some sample config as well:


# dev lifecycle, listens for file changes and reruns appropriate tasks
  watcher:
    svgicons:
      files: ["<%= files.svgicons.watch %>"]
      tasks: ["grunticon", "copy:svgicons", "sass:admin"]

    admin:
      files: ["<%= files.js.admin.watch %>"]
      tasks: ["newer:coffee:admin", "concat_sourcemap:admin", "copy:admin", "copy:sourcemaps"]
davemo commented 10 years ago

This watch task takes directory paths instead of globs, and creates a watch on that directory and recurses into subdirs so that watches occur at all levels of the path.

ie: the icons path, and all subfolders, any files that change/delete/modify trigger this watch task to re-run the configured tasks.

  svgicons:
    watch: ["#{ADMIN_PATH}/resources/icons"]
davemo commented 10 years ago

This watcher powers the shopify dev lifecycle, on an app with over 1800 files being watched, grunt-contrib-watch uses upwards of 40% latent cpu with that many files, our watcher consumes 0.1% cpu and works much better :)

searls commented 10 years ago

Ah, so all you miss out on is the ability to, say, only watch for changes of a single file type under a directory?

That is, if I have JS and Coffee in the same directory, I would lose the ability to change a JS file without kicking off my coffee task?

On Tue, Jan 14, 2014 at 3:43 PM, David Mosher notifications@github.com wrote:

This watch task takes directory paths instead of globs, and creates a watch on that directory and recurses into subdirs so that watches occur at all levels of the path. ie: the icons path, and all subfolders, any files that change/delete/modify trigger this watch task to re-run the configured tasks.

  svgicons:
    watch: ["#{ADMIN_PATH}/resources/icons"]

Reply to this email directly or view it on GitHub: https://github.com/testdouble/lineman/issues/185#issuecomment-32305291

davemo commented 10 years ago

For the most part you are correct, directories in which we have multiple file extensions are by design and are consumed by tasks that expect that (ie: an images dir with .png, .jpg, .gif that are processed by grunt-contrib-imagemin), but yes other than that we don't have multiple file types per directory.

davemo commented 10 years ago

It seemed an appropriate sacrifice to make given the CPU crazy in contrib-watch and watch-nospawn.

10thfloor commented 10 years ago

... The vanilla lineman scaffold (ie. lineman new && lineman run ) still does not have watch/open functionality by default? The version I installed yesterday still has some refs to grunt-contrib-livereload ...

Keep up the good work!

searls commented 10 years ago

Hey @10thfloor -- lineman run out of the box does include watch functionality. We don't include open or livereload, however.

searls commented 10 years ago

Here's what it would take to add grunt-open to lineman core.

Not sure if this is a thing people would want generally enough to pull it into core. Not sure if it does enough to warrant a lineman plugin:

https://github.com/testdouble/lineman/commit/3f4d0d48d1ca70e087a468d802ad96aa2cf65056

ichabodcole commented 10 years ago

If not currently available in lineman core or as a supported lineman plugin, is there a recommended path/plugin to integrating livereload with lineman? The grunt-contrib-livereload is a little off putting as the repository basically redirects you to grunt-contrib-watch.

FYI, I am just getting started with lineman, coming from yeoman, and originally coming from just using grunt alone. I felt a lot of the gaps / concerns with yo that lineman attempts to address, while providing some structure and a good starting feature set. That said as someone who does a lot of frontend design / interface prototyping I really rely on livereload when creating a new interface, and not having it out of the box feels like the lineman pie is missing a slice ;).

If not planning on integrating it into core in the near future, I think it would really help people like myself if you dedicated a little space in the installation guide on the recommended path to adding it.

searls commented 10 years ago

Ooh, I'm glad to hear from someone who wants this. I just spiked a grunt-open plugin a few days ago and was thinking of combining it with livereload. Could you provide a livereload config you like? I've never used it before.

On Mon, Feb 10, 2014 at 1:28 PM, Cole notifications@github.com wrote:

If not currently available in lineman core or as a supported lineman plugin, is there a recommended path/plugin to integrating livereload with lineman? The grunt-contrib-livereload is a little off putting as the repository basically redirects you to grunt-contrib-watch. FYI, I am just getting started with lineman, coming from yeoman, and originally coming from just using grunt alone. I felt a lot of the gaps / concerns with yo that lineman attempts to address, while providing some structure and a good starting feature set. That said as someone who does a lot of frontend design / interface prototyping I really rely on livereload when creating a new interface, and not having it out of the box feels like the lineman pie is missing a slice ;).

If not planning on integrating it into core in the near future, I think it would really help people like myself if you dedicated a little space in the installation guide on the recommended path to adding it.

Reply to this email directly or view it on GitHub: https://github.com/linemanjs/lineman/issues/185#issuecomment-34664443

ichabodcole commented 10 years ago

Not exactly sure if this is what you wanted, but if I was watching for files to reload in lineman it might look something like the below. I pretty much pulled this from the gruntfile in a current Yeoman based project which runs livereload through watch.

files:
[
 'generated/js/{,*/}*.js',
 'generated/{,*/}*.html',
 'generated/css/{,*/}*.css',
 'generated/images/{,*/}*.{gif,jpeg,jpg,png,svg,webp}'
]

Of course you could also just watch and reload on any change in the app folder, or specify watching coffee / js, and less / sass / css, etc. file changes. This might? save some cpu / time vs watching the generated folder.

ichabodcole commented 10 years ago

This lead me to another question. Is there anyway to tap into the core watch plugin in lineman? That is to say if I have a task I want to run when a file changes can I shoehorn that in to the lineman watch cycle?

searls commented 10 years ago

Sure is, just add something like this to your config/application.js file's return object:

watch: {
  newWatchTarget: {
    files: ["app/**/*.blah"],
    tasks: ["blahTask1", "blahTask2"]
  } 
}
ichabodcole commented 10 years ago

Ahh, ok that is very helpful. I may have missed it, but I think it would be helpful to add an example of this in the site under "Customizing Lineman". (Several minutes later) I just went back and was able to find the example which shows task removal and replacement/overwriting. My anecdotal experience was that I missed the example link even after a few read throughs. I think an inline example vs outbound link would really help with that. Additionally the link only says "here's and example of removing a task", which is actually not what I was looking/scanning for. It seems obvious now, but I ended up focusing on the append and prepend objects because the examples were more visible on the site docs. To sum that long paragraph up, I would recommend updating the site docs to include the following.

  1. Inline example of editing/removing a core task.
  2. If not the above, or in addition to it add the keyword "editing" to the outbound link under the "Customizing Lineman" section. Thanks!

p.s. Not sure if there is an online repo for the site, but if so point me to it and I would be glad to submit a PR to add the above.

searls commented 10 years ago

@ichabodcole you're looking for https://github.com/linemanjs/lineman-docs -- and yeah, I'm planning a near-total rewrite of the docs in the next couple months to focus more on user activities and less on feature-listing

ichabodcole commented 10 years ago

@searls excellent, thanks again.

erictaylor commented 10 years ago

I'm just curious what the current status of including LiveReload in some way is? LiveReload has become an essential part of my workflow, and I can't stand not having it. It's been my one and only real hang-up with doing a full move from Yeoman to Lineman. So having LiveReload available in core, or even as a plugin would be a HUGE win in my book.

searls commented 10 years ago

We're not actively working on livereload, but if you want to add it to current versions of lineman, I imagine what it would take is to patch:

https://github.com/testdouble/grunt-watch-nospawn/blob/master/tasks/watch.js

With something that invoked livereload. The task would need to look at a grunt config path (e.g. watch.options.livereload.enabled) to trigger it, and then information about the livereload server and the lineman server (which will be at localhost + watch.server.port)

On Fri, Jun 13, 2014 at 2:17 AM, Eric Taylor notifications@github.com wrote:

I'm just curious what the current status of including LiveReload in some way is? LiveReload has become an essential part of my workflow, and I can't stand not having it. It's been my one and only real hang-up with doing a full move from Yeoman to Lineman. So having LiveReload available in core, or even as a plugin would be a HUGE win in my book.

Reply to this email directly or view it on GitHub https://github.com/linemanjs/lineman/issues/185#issuecomment-45986531.

davemo commented 10 years ago

We had a user from RackSpace integrate LiveReload without needing to make any patches to Lineman core; I'll see if I can find out what she did.

searls commented 10 years ago

Sorry I can't be of more help. I barely know how livereload works.

On Fri, Jun 13, 2014 at 8:54 AM, David Mosher notifications@github.com wrote:

We had a user from RackSpace integrate LiveReload without needing to make any patches to Lineman core; I'll see if I can find out what she did.

Reply to this email directly or view it on GitHub: https://github.com/linemanjs/lineman/issues/185#issuecomment-46020780

erictaylor commented 10 years ago

@searls @davemo Thanks for the update. @davemo Can you look into the user from RackSpace you said might have figured out a solution? It would be a huge time saver if someone else has already solved the problem already. If not I might put some hours in and see what I can get figured out.

davemo commented 10 years ago

@Psiablo, Github user @jewelsjacobs added the livereload snippet to her index.us page and also pointed to the dev branch of lineman-livereload in her package.json file. Hope that helps give you some context :)

erictaylor commented 10 years ago

Well as far as I got was getting connect-livereload added which injects the livereload script into your served html files through the express middleware. I played with stripping out grunt-watch-nospawn and replacing with grunt-contrib-watch, and was able to get a version of lineman and live reload working that way. However, after rereading this thread it seems like switching back to grunt-contrib-watch is out of the question. Patching grunt-watch-nospawn to include live reload is beyond my knowledge and realm.

If anyone is interested in seeing what I had going for injecting the live reload script via the express middleware, you can view my changes in my forked repo.

I realize that not having live reload is not a huge pain point to those who aren't used to working with it, but for someone who does, it's a sorrily missed feature.

I'm hoping that someone with more knowledge of how grunt-watch-nospawn works, will take a moment to see if they can get live reload working.

searls commented 10 years ago

livereload support landed in lineman@0.33.3

erictaylor commented 10 years ago

@searls Awesome! Glad to see some progress has been made.

EDIT: Please see #283 to add the Live Reload middleware to Lineman core which can be enabled via lineman-livereload.

searls commented 10 years ago

I don't believe you do. 

Any time any grunt task would fire via watch, livereload will trigger. If by "express", you mean any routes served from lineman's development server "config/server", that is being reloaded by watch_r and won't typically fire an associate grunt task. That said, changes to the dev server should be much less frequent than changes to app code (since it's not production code). 

Regarding the watch task, the change to grunt-watch-nospawn to support livereload was truly trivial and I'm disappointed no one thought to contribute the backport themselves, instead of just asking for it. Since we don't use livereload, it's unlikely we'll invest time supporting it.

On Sun, Jun 22, 2014 at 4:15 PM, Eric Taylor notifications@github.com wrote:

@searls Awesome! Glad to see some progress has been made. I'll test this out today. Anything I should be aware of? I'm only seeing changes to watch, but not to lineman core, so I'm assuming I still need to put my express middleware in?

Reply to this email directly or view it on GitHub: https://github.com/linemanjs/lineman/issues/185#issuecomment-46794692