browserify / watchify

watch mode for browserify builds
Other
1.79k stars 181 forks source link

Watchify not noticing changes when symlinks used #109

Closed nnarhinen closed 9 years ago

nnarhinen commented 9 years ago

cd node_modules && ln -s ../lib/frontend

watch.js:

var browserify = require('browserify'),
    watchify = require('watchify'),
    fs = require('fs'),
    bundler;

bundler = watchify(browserify('./src/frontend/index.js', {
  basedir: __dirname + '/../',
  debug: true,
  cache: {}, 
  packageCache: {}, 
  fullPaths: true
}));

bundler.transform('reactify');

bundler.on('update', function() {
  var wb = bundler.bundle();
  wb.on('error', function(err) {
    console.error(err.message, err.trace);
  }); 
  wb.pipe(fs.createWriteStream(__dirname + '/../src/backend/public/js/application.js'));
});
bundler.bundle().pipe(fs.createWriteStream(__dirname + '/../src/backend/public/js/application.js'));

if I modify something under lib/frontend/some/path.js watchify does not rebundle.

If I remove the symlinks and mangle with NODE_PATH instead everything works as supposed.

Am I doing something wrong?

Using OSX btw.

bendrucker commented 9 years ago

See https://github.com/Nikku/karma-browserify/issues/49#issuecomment-63104556 and the general discussion on that issue. chokidar doesn't support symlinks so watchify does not either.

nnarhinen commented 9 years ago

Ok, maybe watchify should use something else than chokidar, looking at the issues in this repo.. And the issue about symlinks (https://github.com/paulmillr/chokidar/issues/31) has been open since 2012, I wouldn't hold my breath about it. @paulmillr any comments here?

paulmillr commented 9 years ago

@es128

paulmillr commented 9 years ago

at the issues in this repo issues

Which issues in particular? Chokidar recently got into a very active dev mode, we fixed a lot of bugs. It's rock-solid now.

es128 commented 9 years ago

I will be delving into the symlinks stuff for chokidar very soon. I expect to have pretty good solutions within a couple weeks.

nnarhinen commented 9 years ago

Well I looked at https://github.com/substack/watchify/issues/82#issuecomment-52112897 just earlier, but seems maybe that particular issue could be closed by upgrading chokidar dependency.

paulmillr commented 9 years ago

Yep, it was solved.

mattdesl commented 9 years ago

Oddly enough, using grunt-browserify@2.1.4 with watch: true seems to follow symlinks. Upgrading to newer versions of grunt-browserify (and thus newer versions of watchify) breaks symlinks.

ingro commented 9 years ago

I can confirm that upgrading manually the version of chokidar in watchify's package.json fixed the problem.

MylesBorins commented 9 years ago

It seems like the source of this problem is https://github.com/paulmillr/chokidar/commit/bb1531aa6f0ce1ae4cf8406df0547edaca9039b2

On OSX updating to the latest version of chokidar is still broken for both files in $TMPDIR and for symlinks. This is causing the entire test suite for watchify to halt

I have gone and taken a look at the history, and it would appear that commit https://github.com/paulmillr/chokidar/commit/d323768e9ef317b1e1671779004c63d2610fcec4 works as expected.

es128 commented 9 years ago

What @TheAlphaNerd has uncovered is actually a different issue - when there is a symlink somewhere in the path structure leading up to the project being watched. Symlinks within a project tree did not get seamlessly followed before that commit either (when using fsevents).

The reason reverting to older versions of chokidar fixes the issue is because the default underlying watch mode on OS X for those versions is polling (fs.watchFile), and on current versions it is fsevents. Even with latest chokidar, setting the useFsEvents: false option should resolve the symlink issues, but watchify does not currently provide a way for users to modify which options get passed to chokidar.

Nonetheless, this needs to be fixed in chokidar's fsevents mode so that default settings just work as seamlessly as possible, and I'm working on resolving both flavors of the symlink issue.

es128 commented 9 years ago

https://github.com/paulmillr/chokidar/commit/138c59f42d8f5b5e01787c0d73465fee133817ea resolves the issue with watchify's test suite

drewhamlett commented 9 years ago

I'm surprised more people are not talking about this. It seems pretty standard to use symlinks to prevent doing

var user = require('../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../user');  

Also, putting the app in node_modules doesn't feel right to me.

When can we expect to be able to pass chokidar properties into watchify? Right now I just copied watchify into my project and set useFsEvents: false

es128 commented 9 years ago

Symlinks are now properly supported across all watch methods in chokidar's master branch, so it shouldn't be necessary to revert to polling. Release to come soon.

Feedback welcome from anyone wishing to give it a try in the meantime

cd node_modules/watchify
npm i paulmillr/chokidar

Regardless, I would still advocate for watchify to provide an interface allowing users to pass in their own chokidar options.

scamden commented 9 years ago

@es128 does the bump to 0.12 cover this? if so can someone bump watchify's chokidar version? we're dying to have this functionaliy

es128 commented 9 years ago

chokidar 0.12 does provide much better symlink support. It would be great if the participants in this thread could install it over the version that comes with watchify in their own projects and report back regarding how it goes.

If there are no problems I'm sure it will help to persuade the watchify maintainers to bump.

scamden commented 9 years ago

@es128 i have done as you said successfully. i'm currently using an overridden version of chokidar from master in both plain watchify and in karma-browserify's version. no issues so far.

ghost commented 9 years ago

updated to the latest chokidar, browserify in 2.2.0

scamden commented 9 years ago

@substack after installing with the latest update i have no src files, gettingCannot find module 'watchify', it looks like index.js is missing

scamden commented 9 years ago

apologies, i may have spoken too soon this looks like a more intermittent npm failure..