Skip to content
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

New watching solution #125

Closed
sokra opened this issue Nov 28, 2013 · 57 comments
Closed

New watching solution #125

sokra opened this issue Nov 28, 2013 · 57 comments

Comments

@sokra
Copy link
Member

sokra commented Nov 28, 2013

(see #122)

The watching implementation is far from perfect. Implement a new watching module and use it in webpack.

The new watching module should have to following properties:

  • Watcher should be applied backdated. Solve this by reading filestamps and compare them with the wished start time.
  • It should watch files and directories. For files changing content is relevant. For directories changing the contained files is relevant (renaming counts too).
  • Emit events for changed stuff.
  • Emit a delayed event, which is emitted when for a specified time no additional change occures.
  • pause: Don't emit event anymore, but keep the watchers alive. When starting again less timestamps need to be read and less watchers need to be attached. This should make the whole stuff more stable.
  • close: Really close everything.
  • There is a limit for file watchers, so don't watch every file, but only watch the parent directory. This reduces the watcher count. It must be the direct parent directory. A directory watcher may result in many attached watchers for each child directory. After a change in a directory the timestamps of the files are checked (no every os tells you the filename, but if it do then use it).
  • persistant option (because it is easy)
  • Don't crash on file delete
  • Multiple watchers can share fs watchers
  • Cope with decreasing timestamps (branch switch, etc.)
  • It should have a cool name (wootcher, watchtress, witcher?)

UPDATE: Try it with:

plugins: [
  new webpack.NewWatchingPlugin()
]
@jhnns
Copy link
Member

jhnns commented Nov 28, 2013

  • It should have a cool name (wootcher, watchtress?)

Very important 👍

Maybe witcher? 😉

@nick-thompson
Copy link

Definitely interested in this, any updates? Watching seems a little broken right now for me :/

@jhnns
Copy link
Member

jhnns commented Dec 12, 2013

Well, actually it's not broken, but it behaves kind of weird. But as discussed in #122 it's not as easy as it may seem.

@sokra
Copy link
Member Author

sokra commented Dec 13, 2013

No updates here. This is a very work intensive (and boring) task and my time is limited. The current solution is "ok" so this has a low prio.

@dashed
Copy link
Contributor

dashed commented Jan 10, 2014

I can help with the name =] My suggestion: watchpack

@jhnns
Copy link
Member

jhnns commented Jan 10, 2014

Nice 👍

@fresheneesz
Copy link
Contributor

Question: why aren't you using a watcher module that has already been built? I use chokidar for keeping files in sync between my windows shared folder in a vagrant Centos machine and a non-shared folder (the shared folder causes weird behavior, so i have to copy all the files somewhere else). Chokidar has been pretty robust in my experience.

@jhnns
Copy link
Member

jhnns commented Jun 23, 2014

files in sync between my windows shared folder in a vagrant Centos machine and a non-shared folder (the shared folder causes weird behavior, so i have to copy all the files somewhere else).

Interesting approach, I'm having issues with the shared folder as well.

@fresheneesz
Copy link
Contributor

Well, if you need a head start, here's the utilities I wrote to help me create a watcher process: http://www.btetrud.com/files/fileWatcherUtils.js . Example:

var windowsPathBase = '/vagrant'
var centosPathBase = "/home/vagrant/"+config.mainDirectory

var ignoreList = [/\.git/, /\.idea/, /\.vagrant/, /\.iml/, /Vagrantfile/, /CentOS-6.4-x86_64-v20130427.box/]
var ignore = function(path) {
    var result = false
    ignoreList.forEach(function(pattern) {
        result = result || pattern.test(path)
    })
    return result
}

// windows -> centos
fileWatcherUtils.copyThenSync('/git', ignore, windowsPathBase, centosPathBase)

// centos -> windows
fileWatcherUtils.copyThenSync('/git/scripts/generated/', ignore, centosPathBase, windowsPathBase)

@jhnns
Copy link
Member

jhnns commented Jun 25, 2014

Thx, I'll take a look!

@chanon
Copy link

chanon commented Aug 4, 2014

I'd also like to recommend using chokidar as it is pretty robust. I'm also using a shared vm folder and webpack --watch by itself didn't pick up any changes at all.

I've actually done a simple adhoc patch myself.

In NodeWatchFileSystem.js

At the top
var chokidar = require("chokidar");
(add chokidar to dependencies)

Then around line 130

  async.forEach(items, function processItem(item, callback) {
    var isRunning = false;
    var isScheduled = false;
    item.watcher = chokidar.watch(item.path, {persistent: true, interval: 2000, binaryInterval: 2000});
    item.watcher.on('change', function() {
      if(isRunning) return isScheduled = true;
        isRunning = true;
        readStat(item, done);
    });

As you can see, I just changed the fs.watch to use chokidar.watch / watcher.on('change').

Didn't change anything else as I'm not sure what else should be changed. Just wanted a working solution for my development needs and it looks like its working.

@jhnns
Copy link
Member

jhnns commented Aug 4, 2014

Does chokidar work with shared folders? Is chokidar supporting all requirements as described by sokra? Or are they obsolete @sokra ?

@sokra
Copy link
Member Author

sokra commented Aug 4, 2014

I tried to implement a solution in chokidar, but it has some problems on windows...
When the watched directory is removed it throws an uncatchable exception. Maybe it needs a PR...

@JannesMeyer
Copy link

Another approach would be to use an external tool for file watching. You could completely remove file watching from webpack and instead allow incremental builds. As soon as something on disk changes the external tool could just call webpack again.

For an example of an external file watching tool take a look at watchman (*nix only). It's extremely well-tested and so far doesn't consume a lot of cpu on my machine. (Another example: JetBrains WebStorm).

So far all of the file watching features that are part of build tools and precompilers like grunt, gulp, webpack, browserify, coffeescript, less, etc. have disappointed me. This is an extremly hard problem to solve and I believe it should not be part of any build tool itself.

Edit: I forgot to add why chokidar is not such a great solution at the moment. On OSX it depends on the fsevents module which isn't working anymore since Node 0.11.13 (still in beta, but it will become a problem once that node version becomes stable). The fsevents module was last updated 6 months ago.

@jhnns jhnns mentioned this issue Aug 4, 2014
@fresheneesz
Copy link
Contributor

@JannesMeyer I like that idea a lot. Webpack's watcher has impressed me, but I think it would be good to factor it out so it can be used for other projects, or so that other watcher modules can be used in cases where webpack's watcher isn't sufficient.

Separating it out would also open up other abilities to webpack. For example, if you did this right, someone could devise a way to save the webpack representation of its build state so that one could save it to disk, and use it in a new build that has some other method of figuring out which files have changed.

This would require some way to externalize a representation of whatever state needs to exist to do incremental builds (if that info doesn't exist in the webpack bundle itself) and the ability to specify which modules changed that need to be rebundled. I doubt it'd be difficult to do, since its already done internally in webpack.

@nick-thompson
Copy link

@fresheneesz agreed 100%. I had the exact same thought :). This would be awesome.

@sokra
Copy link
Member Author

sokra commented Aug 5, 2014

@fresheneesz #250

I've started a new library that should handle watching in the future, but I wasn't able to complete it yet.

The design was to never watch files directly, but always watch the containing folder. This way the required watcher count is reduced. In addition to that multiple watchers on the same directory are merged to futher reduce the watcher count.

The API allows to pause a Watcher, which doesn't close the internal watchers. This makes it cheap to restart the Watcher and allows to tell if there was an event in the past (which eliminates unnecessary recompilation).

text9240

Currently I'm blocked by chokidar not working correctly...

@lencioni
Copy link
Contributor

While better built-in watcher would be nice, I think it would be great if I could run a command to tell webpack which files have changed and then it can do the incremental rebuild that way. This would allow me to compose webpack with whichever watcher I like, such as watchman, simply piping lists of changed files to webpack.

Something like webpack --changed-files file1.js file2.js

I don't have any knowledge about how webpack internals work or what is involved in writing a good watcher, but it seems like this should be relatively straightforward to do compared to writing a watcher, and it would give people potentially better options while the built-in watcher is rebuilt. What do you think?

@sokra
Copy link
Member Author

sokra commented Aug 18, 2014

@lencioni I'll check how we can add an API to support any watcher.

chokidar issue seem to be fixed...

@madebyherzblut
Copy link

We had a few problems with shared folders a while ago. Because of that we now use rsync with vagrant-gatling-sync and it works great. Of course, for one way syncing only.

@ghost
Copy link

ghost commented Feb 19, 2015

Yeah, I tried that, but I need two way syncing.... Luckly, I found unison, a sync tool via ssh. It works very fast, faster than rsync I guess.... Well, I'm using it and it's working 💃

Thanks for your answer!

@ColCh
Copy link

ColCh commented Feb 19, 2015

@EricPrieto If you're looking for perfect Vagrant dir sharing, try to share "current work dir" with 2 mount point - one for rsync and one for usual vboxsf. That gives just perfect workflow with file sharing...

@fresheneesz
Copy link
Contributor

@EricPrieto Could you elaborate a bit on that? I'm pretty unhappy with way's I've found to do a shared folder in vagrant. Is there somewhere you could link to that describes that in more detail? I'll also have to look at unison - i built my own ad hoc watcher that's actually working pretty well at this point, but could be faster.

@rarkins
Copy link

rarkins commented May 22, 2015

@fresheneesz I documented how we install/use unison with vagrant manually: https://keylocation.sg/blog/vagrant-and-unison-without-a-plugin/

@ghost
Copy link

ghost commented May 26, 2015

After a few weeks trying unison and every other solution, I just gave up.

@SpaceK33z
Copy link
Member

@EricPrieto As of webpack 1.9.10 you can use webpack --watch --watch-polling (see 297707d). This works perfectly fine over NFS and in combination with Vagrant.

@ghost
Copy link

ghost commented Jun 6, 2015

Awesome!
On Jun 6, 2015 11:19 AM, "Kees Kluskens" notifications@github.com wrote:

@EricPrieto https://github.com/ericprieto As of webpack 1.9.10 you can
use webpack --watch --watch-polling (see 297707d
297707d).
This works perfectly fine over NFS and in combination with Vagrant.


Reply to this email directly or view it on GitHub
#125 (comment).

@Stas404
Copy link

Stas404 commented Jul 6, 2015

SpaceK33z, thanks.
--watch-polling

@alanbacon
Copy link

I found "--watch-polling" doesn't work. Instead I had to use "--watch-poll".... it's a small thing but nevertheless a source of confusion. I'm running webpack 1.11.0 and webpack-dev-server 1.10.1

@3oheme
Copy link

3oheme commented Sep 22, 2015

@alanbacon --watch-poll also works great for me, thanks!

Macbook pro late 2011
Iterm2 Build 2.1.1
Vagrant 1.7.2
NPM 1.4.28

@piecyk
Copy link

piecyk commented Feb 1, 2016

Hi i'm getting some strange behavior and don't know what is wrong, using webpack 2.0 on VM with Arch Linux, running webpack --watch --watch-poll
everything is good till (lint works on every file change) i got some Module build failed error. For example f ( {} and save this.
Will throw an error from babel-loader and after fixing it it will not reload the file. Watch is stuck! need to change dir to diffident component (modifies something there, and hoping that it will work)
I added the fs.inotify.max_user_watches=524288 but didn't help...

Has some seen something similar? this is my config ( tried so many configuration but nothing helped ) and don't know if it's webpack or babel-loader problem ?

var path    = require('path');
var webpack = require('webpack');

//
var gulpUtil = require('gulp-util');
var env = gulpUtil.env;
var isProduction = env.env === 'production';

module.exports = {
  debug: !isProduction,
  resolve: {
    modules: [
      path.resolve(__dirname, 'src')
    ],
    alias: {
      lib: path.resolve(__dirname, 'src/lib')
    },
    extensions: ['', '.js', '.jsx', '.json'],
    unsafeCache: true
  },
  entry: {
    global: './src/a/global.js',
    pub: './src/a/site/js/pub.js',
    app: './src/a/app.js'
  },
  output: {
    path: path.join(__dirname, 'build/js'),
    filename: '[name].js',
    publicPath: '/build/js/'
  },
  plugins: [
    new webpack.optimize.CommonsChunkPlugin({
      name: 'commons',
      chunks: ['pub', 'global', 'app'],
      minChunks: 2 // How many times a dependency must come up before being extracted
    }),
    new webpack.DefinePlugin({
      "process.env": {
        "NODE_ENV": JSON.stringify(env.env)
      }
    }),
    new webpack.ProgressPlugin()
  ].concat(isProduction ? [
    // This plugin looks for similar chunks and files
    // and merges them for better caching by the user
    new webpack.optimize.DedupePlugin(),
    // This plugin prevents Webpack from creating chunks
    // that would be too small to be worth loading separately
    new webpack.optimize.MinChunkSizePlugin({
      minChunkSize: 51200 // ~50kb
    }),
    // This plugin minifies all the Javascript code of the final bundle
    new webpack.optimize.UglifyJsPlugin({
      mangle: true,
      showStack: true,
      compress: {
        drop_console: true
      }
    })
  ] : [
    new webpack.SourceMapDevToolPlugin({
      filename: '[file].map',
      exclude: ['global', 'pub'],
      columns: false
    })
  ]),
  module: {
function(errors) { gulpUtil.log("\n" + errors); }] : null))
    preLoaders: [
      {
        test: /\.(js|jsx)$/,
        loader: 'eslint-loader',
        include: path.resolve(__dirname, 'src'),
        exclude: [
          path.resolve(__dirname, 'src/lib/vendor'),
          path.resolve(__dirname, 'src/lib/router.js'),
        ]
      }
    ],
    loaders: [
      {
        test: /.(js|jsx)?$/,
        loader: 'babel-loader?cacheDirectory',
        include: path.resolve(__dirname, 'src')
      },
      {
        test: /\.json$/,
        loader: 'json-loader'
      }
    ]
  }
};

@ColCh
Copy link

ColCh commented Feb 1, 2016

@piecyk you should ask this on SO, probably.

Try to set larger watch timeouts:

watchOptions: {
    aggregateTimeout: 800,
    poll: 1500
}

It's in webpack-dev-server's config, please see more in docs

@piecyk
Copy link

piecyk commented Feb 1, 2016

@ColCh it's not a problem of timeouts, watch is working good till i hit the first Parsing error
then it is stuck... hmm i think something is wrong around babel-loader

@ColCh
Copy link

ColCh commented Feb 1, 2016

@piecyk Try then inlude NoErrorsPlugin and specify babel-loader presets in webpack config also, as query (I had problems with .babelrc and latest loader version).

If it will not help... I don't know. May be a bug also.

@piecyk
Copy link

piecyk commented Feb 1, 2016

From quick debug it looks like when file has error it's removed form the list of files to be watched,

NodeWatchFileSystem.prototype.watch = function watch(files, dirs, missing, startTime, options, callback, callbackUndelayed) {

here files don't have this file, event if we fix it and re run it by changing another file it will not reload it... i think cache is taking over here...

@StephanBijzitter
Copy link

Old watching plugin or new, they both do not pick up on change events.
Running OS X Yosemite, with a Ubuntu 14.04 VM.

When I run the watcher on OS X, all is fine. However, the reason we use a VM (through Vagrant) is to streamline the process. Bit of a bummer. Polling does not seem to work, either.

@jhnns
Copy link
Member

jhnns commented Feb 6, 2016

VirtualBox's shared folder does not trigger change events, so this is not an issue of webpack (further discussion #425). I'm using rsync to update the changed files in the VM which works great.

@ColCh
Copy link

ColCh commented Feb 6, 2016

@jhnns event get not triggered only with vboxfs or with NFS too?

@jhnns
Copy link
Member

jhnns commented Feb 7, 2016

@ColCh Don't know

@StephanBijzitter
Copy link

@jhnns When the polling support in webpack also fails, it does make it a webpack problem

@NickStefan
Copy link

Just moved to using a vagrant + NFS.

  • regular build uses the webpack CLI to watch. it wasnt watching changes correctly with vagrant. I turned on polling and it now works flawlessly.
  • testing framework wraps webpack's node API and mocha. I turned on polling. it now triggers the watch event correctly (and re-runs our tests). however, the actual output file is not being rebuilt.

What could be the difference between the node API and the cli? its the only difference I can think of.

@StephanBijzitter
Copy link

Just read through all of this, and I must say that it would be great to remove the watcher from webpack entirely. Indeed, we could rely on external packages to watch for us if we get the incremental compilation right! It would reduce maintenance for the webpack team too, it's a win-win!

@sap9433
Copy link

sap9433 commented Nov 13, 2017

I think having a plugin to watch external files is a better approach . How does this look ? https://www.npmjs.com/package/filewatcher-webpack-plugin .

@felipenmoura
Copy link

This is all very interesting.
Is there a way I can/could identify when a watch event was triggered due to a new file, or to a "file changed"?

KristoffAlaric added a commit to KristoffAlaric/overcommit that referenced this issue Aug 3, 2024
We have been using Webpack which has a built-in file watcher.

When using the Webpack watcher along with overcommit, we were noticing
that after every commit we were making, the compiled Webpack bundle
would not contain the changes from that commit.

By default, this watcher has a 200ms delay on file changes before it
actually starts rebuilding, which is good enough for standard
development. However, because of the way that overcommit plays with the
working tree, our Webpack bundle was getting compiled without the
changes from our most recent commit.

This is happening because Webpack looks at the modified times of the
files to determine if something has changed. If the modified time of the
file is greater than the time that Webpack started watching, it
registers as a change. Since overcommit is changing the modified times,
this gets messed up.

My theory is that the amount of time that elapses from the start of
cleaning up the environment until the end of cleaning it up is greater
than our Webpack watchDelay.

We could probably resolve this by raising the watchDelay, but that slows
down our standard development which I would prefer to avoid doing.
Instead, it seems that if we restore the file modification times after
every step when cleaning up the environment in overcommit, we can have a
lower watchDelay without the negative side-effects described above. I do
realize that this is not a full fix, but it should help.

Webpack is currently working on building a new watcher [1], which may
also resolve this issue for us, but I believe that this change will also
likely benefit other file watchers.

When restoring the modified times, we need to now make sure that the
file exists because we are restoring after the working tree is cleared,
so if the file is new, it will not exist (which will cause it to blow
up).

[1]: webpack/webpack#125

Change-Id: Ia28d6952bac5ef0d1ccac2276df2b62c46f11f68
Reviewed-on: http://gerrit.causes.com/44132
Tested-by: jenkins <jenkins@causes.com>
Reviewed-by: Shane da Silva <shane.dasilva@brigade.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests