Parcel: [Feature] Automatically detect safe-write

[edo-codes]

🐛 Bug report

While the parcel dev server or the watcher is running, changes in a file are sent to the browser correctly, but only the first time I save changes in the file. The second time, parcel doesn't rebuild (there is no console output) and the module doesn't reload in the browser. Other files will still reload, but also only once.

Even when refreshing in the browser, the changes aren't shown. I have to restart parcel for it to show the latest changes again.

Reproduction

Here's a minimal sample that shows the problem for me: https://github.com/Edootjuh/parcelrepro

To reproduce:

• Start parcel with parcel index.html
• Load http://localhost:1234 in browser. The page shows a b
• Edit module-a and save. The page changes. Edit it again. The page doesn't change.
• Edit module-b and save. The page changes. etc.

The same happens with parcel watch index.html

🌍 Your Environment

| Software | Version(s)
| ---------------- | ----------
| Parcel | Latest on master (commit 2d680c0)
| Node | node v8.9.1
| npm/Yarn | yarn 1.2.1
| Operating System | Ubuntu 17.10

[davidnagli]

I was able to reproduce this on Mac OS as well. However, the problem goes away when I remove module.hot.accept

[edo-codes]

That's weird. I just tried without module.hot.accept, and the problem is still there for me. (I've cleared the cache)

[davidnagli]

Can you commit the new code (the code without module.hot.accept) to a new branch in your example repo?

[edo-codes]

I'd just changed the code in the example repo to remove module.hot.accept in the master branch before your first comment, so if you pull/reset again it should update

[DeMoorJasper]

I can't seem to reproduce this in any way

[edo-codes]

I've done some poking around, and it seems like chokidar isn't firing change events for the subsequent edits in Bundler.js, but it does when I set usePolling to true. Obviously that isn't a fix, since that option results in poorer performance, but maybe that helps?

diff --git a/src/Bundler.js b/src/Bundler.js
index bb21004..cd0cf1f 100644
--- a/src/Bundler.js
+++ b/src/Bundler.js
@@ -176,7 +176,8 @@ class Bundler extends EventEmitter {
       // FS events on macOS are flakey in the tests, which write lots of files very quickly
       // See https://github.com/paulmillr/chokidar/issues/612
       this.watcher = new FSWatcher({
-        useFsEvents: process.env.NODE_ENV !== 'test'
+        useFsEvents: process.env.NODE_ENV !== 'test',
+        usePolling: true
       });

       this.watcher.on('change', this.onChange.bind(this));

[edo-codes]

I've found the problem. It has to do with the way many text editors write files, and is actually addressed by the webpack documentation: https://webpack.github.io/docs/webpack-dev-server.html#working-with-editors-ides-supporting-safe-write. To quote:

Working with editors/IDEs supporting “safe write”

Note that many editors support “safe write” feature and have it enabled by default, which makes dev server unable to watch files correctly. “Safe write” means changes are not written directly to original file but to temporary one instead, which is renamed and replaces original file when save operation is completed successfully. This behaviour causes file watcher to lose the track because the original file is removed. In order to prevent this issue, you have to disable “safe write” feature in your editor.

• VIM - set :set backupcopy=yes (see documentation)
• IntelliJ - Settings ▶︎ System Settings ▶︎ Synchronization ▶︎ disable safe write (may differ in various IntelliJ IDEs, but you can still use the search feature)

I suppose the reason I'd never had this problem before is because I'd always configured webpack to watch the _directory_ instead of individual files.

This means this isn't really a parcel bug, but it is likely an issue that other people will face as well (I've tried editing the file with IntelliJ IDEA, vim and gedit, and they all behaved the same), so it might be worth adding an extra recursive watcher for e.g. the root directory so you can receive events for these 'new' files and rebuild.

[davidnagli]

Wow thanks for hunting this down! I would have never figured that out.

Do you guys think we should add a warning about this in the documentation?

[DeMoorJasper]

We should maybe add an option to the cli to enable polling for the people who have this activated in their IDE? (because this isn't really an option as standard for performance reasons)
And document it

[edo-codes]

Personally, I think watching the directory would solve it completely (though I could be wrong), but in the meantime adding a warning and a cli flag for usePolling seem like good options to me.

[DeMoorJasper]

isn't use polling poking the files every now and then, you might be right though?

[davidnagli]

Is there a way to detect “safe write”? Maybe we can dynamically fallback to polling when we detect it.

[edo-codes]

When you look at the raw fs events from chokidar, at least on Linux, on a safe-written file change you get a change event, and then two rename events (as opposed to a single change event).

[DeMoorJasper]

@Edootjuh Have you tried listening for the 'add' event, this would trigger if a new file gets written what happens with safe write i guess?
I can't unfortunately test any of this myself due to using mac OS, would be cool to have a fix in place that doesn't require any additional configuration or performance issues
Edit: this didn't work

[juniovitorino]

Here using VSCode the same behavior happen, VSCode + OS X High Sierra, HRM only works once.

[DeMoorJasper]

This is an IDE related issue @juniovitorino turn off safe-write and it should work

[kasbah]

I just encountered this issue as well. I had never encountered it before with anything else and I use file watchers all the time. Seems like it should be fixable without making users change their IDE settings.

• I found this commit to meteor-hmr which could provide some insight.

• We could also look at the gaze filewatcher which seems to handle safewrites since version 0.3

Also, there seems to be a regression since 1.4.1 (at least for Vim). What changed for file watching?

[DeMoorJasper]

@kasbah do you perhaps have time to tackle this issue or write a test that mimics the safewrite behaviour so we can find a solution for this?

I gave it a go earlier and came up with this test, it's not doing what i want though as it passes on the current master.

it('Should detect safewrite', async function() {
    await ncp(__dirname + '/integration/safewrite', __dirname + '/input');

    b = bundler(__dirname + '/input/index.js', {
      watch: true
    });
    let bundle = await b.bundle();
    let output = run(bundle);
    assert.equal(output(), 3);

   const file = __dirname + '/input/local.js';
   const backup = __dirname + '/input/local.js~';
   function simulateSafewrite(content) {
      fs.writeFileSync(backup, fs.readFileSync(file));
      fs.writeFileSync(backup, content);
      fs.unlinkSync(file);
      fs.renameSync(backup, file);
    }

   simulateSafewrite('exports.a = 5; exports.b = 5;');

    bundle = await nextBundle(b);
    output = run(bundle);
    assert.equal(output(), 10);
  });

Also tried adding some changes to bundler, to listen for folders instead of files, but not sure if that works as my test is not really perfectly mimicking the behaviour

[Grafikart]

I got the same problem.
@DeMoorJasper idea is good (writing a test that fails) but I am unable to "simulate" safe write too to make a failing test. Maybe safe write mess with the file properties ?

[tatsujb]

I'm not sure this is the right place (I'm running angular 5) but my setup : this one does not auto rebuild when I edit and save a file. I don't understand. isn't that supposed to be default or do I have to specify something for that to happen?

[DeMoorJasper]

@tatsujb if u are using safe-write this sorta is the right place you can have a look at the docs at how to disable this.
https://en.parceljs.org/hmr.html#safe-write

[tatsujb]

errrrm ....this link should be within the Parcel's README.MD it's vital info x) thanks! (webstorm user)

[DeMoorJasper]

@tatsujb It is inside the readme See parceljs.org for more documentation!

[tatsujb]

I meant that specific bit but fair enough.

[frhagn]

I'm also having problems with safe-write when using Visual Studio 2017 combined with parcel watch.
The problem with Visual Studio is that safe-write can't be turned of..

From what I can tell the safe-write process in Visual Studio looks like this when saving a file (file.js in this example):

1. Create an empty temp file. 5jrvappx.wht~
2. Write the content to the temp file. 5jrvappx.wht~
3. Create a second empty temp file. file.js~RF46beda4b.TMP
4. Delete the second temp file. file.js~RF46beda4b.TMP
5. Rename the original file to the same filename as in step 3. file.js > file.js~RF46beda4b.TMP
6. Rename the first temp file from to the original filename. 5jrvappx.wht~ > file.js
7. Delete the temp file from step 5. file.js~RF46beda4b.TMP

@DeMoorJasper @kasbah maybe this will break the test:

   const temp = __dirname + '/input/5jrvappx.wht~';
   const file = __dirname + '/input/local.js';
   const backup = __dirname + '/input/local.js~RF46beda4b.TMP';
   function simulateSafewrite(content) {
      fs.writeFileSync(temp, content);
      fs.renameSync(file, backup);
      fs.renameSync(temp, file);
      fs.unlinkSync(backup);
    }

[mitar]

I would also add that coming from Meteor which works well with editors which use safe write, this seems like something which should be fixable and it should not require disabling safe write, which is at most a workaround and cannot be seen as simply a solution.

[mitar]

See here relevant code in Meteor.

[mitar]

Some related issues:

• https://github.com/paulmillr/chokidar/issues/742
• https://github.com/paulmillr/chokidar/issues/237

It seems chokidar has option atomic which we could try to make it work correctly when it is set to true: https://github.com/paulmillr/chokidar#errors I have tested it and it seems it does not work.

So it seems we should try to fix chokidar here, not parcel.

[mitar]

I think I fixed this. If you want to test it, just install git://github.com/mitar/chokidar.git in your app instead of the original chokidar.

See https://github.com/paulmillr/chokidar/pull/791 for more details.

[DeMoorJasper]

@mitar awesome work, hope it'll get merged into chokidar 🎉

[mikolajpp]

@mitar I have tried to install chokidar from your repo, as a fix for #2661, but it unfortunately does not work, at least in neovim. Only the first write is detected, afterwards reloading does not work.

[mitar]

Strange. It works great for me for PyCharm. Make sure you really use that forked version and that it npm does not restore to previous version.

So I added to my package.json:

  "devDependencies": {
    "chokidar": "git://github.com/mitar/chokidar.git"
  }

And it works then.

[mikolajpp]

I have copied your entry verbatim.
Now my package.json looks like this:

{
  "dependencies": {
    "parcel-bundler": "^1.11.0"
  },
  "devDependencies": {
    "[chokidar](https://npmjs.com/package/chokidar)": "git://github.com/mitar/c$
    "elm-hot": "^1.0.1",
    "node-elm-compiler": "^5.0.3"
  }
}

Still, hot reload works only on the first edit in neovim. Afterwards it is broken.

[mitar]

Sorry. Not sure why there was that Markdown in JSON. I edited my comment before accordingly.

[mikolajpp]

Ah right. Still, I have fixed this, removed node_modules, did a reinstall and it is still broken.
Is there any way I can verify that the modified chokidar is indeed used/run by parcel?

[mikolajpp]

I have installed chokidar-cli, which I presume uses the version from your repo, and running
it from the command line reproduces the same behaviour in neovim. First write is detected, subsequent writes are not.

[mitar]

My PR has been merged into upstream chokidar.

[DeMoorJasper]

Should be fixed in Chokidar, closing this issue.

Feel free to open a new issue if this persists. (Do note that this fix has not been released yet and might take some time before your node_modules are up to date with the chokidar fix.)

[mitar]

Shouldn't then this issue be closed once chokidar is released with that fix and parcel updates its package.json to point to it?

Also, before closing this issue, documentation for parcel should be updated to not advise anymore to disable safe write.

[DeMoorJasper]

@mitar if it’s a minor release the package.json shouldn’t be updated although we probably will. But docs should probably wait untill chokidar had the update.

_{Sent with GitHawk}

[cmcaine]

chokidar reverted @mitar's workaround because it broke another tool and this is broken again.

https://github.com/paulmillr/chokidar/commit/b79120bcd5be17b300559c6fa292ab25e5164070

My setup:

$ yarn list parcel chokidar
yarn list v1.16.0
warning Filtering by arguments is deprecated. Please use the pattern option instead.
├─ [email protected]
└─ [email protected]
Done in 0.74s.

Writing a tiny chokidar script shows that chokidar isn't seeing the changes when it's watching particular files:

const chokidar = require('chokidar')

chokidar.watch('src/content.ts').on('change', console.log)

Chokidar sees the first change to the file, but not the second and subsequent. Watching the directory instead fixes this. I think parcel should watch the whole directory and filter names on linux. This gives a quick fix that won't break any other tools.

[cmcaine]

Obviously another fix would be to just pin chokidar at 2.1.3 or 2.1.4 for now.

I'm doing that locally with:

// package.json
  "resolutions": {
    "parcel/**/chokidar": "2.1.3"
  }

Which is a yarn feature: https://yarnpkg.com/en/docs/selective-version-resolutions