Can we avoid calling fs.realpathSync(filepath) if filepath is already an absolute path?

[rmacklin]

I ran into an issue trying to use this plugin in my gulp watch task.

I have this code:

const gulp = require('gulp');
const sassGrapher = require('sass-graph');
const touch = require('touch');

const sassGraph = sassGrapher.parseDir('./app/assets/stylesheets');

gulp.task(
  'watch',
  () => {
    // ... (additional watch statements)

    // Trigger recompilation of stylesheet entry points when their dependencies are modified
    gulp.watch([buildStylesTask.watchFiles], (event) => {
      sassGraph.visitAncestors(event.path, (parent) => {
        if (parent.includes('stylesheets/packs')) {
          touch.sync(parent);
        }
      });
    });
  }
);

For the most part, this works pretty well, except if event.path refers to a watched file that was deleted. In that case, the watch task crashes with:

Error: ENOENT: no such file or directory, lstat '/Users/rmacklin/src/m/app/assets/stylesheets/components/tooltip.scss'
    at Object.realpathSync (fs.js:1657:15)
    at Graph.visit (/Users/rmacklin/src/m/node_modules/sass-graph/sass-graph.js:117:17)
    at Graph.visitAncestors (/Users/rmacklin/src/m/node_modules/sass-graph/sass-graph.js:101:8)
    at gulp.watch (/Users/rmacklin/src/m/lib/tasks/assets/watch.js:18:56)
    at Gaze.<anonymous> (/Users/rmacklin/src/m/node_modules/glob-watcher/index.js:18:14)
    at emitTwo (events.js:126:13)
    at Gaze.emit (events.js:214:7)
    at Gaze.emit (/Users/rmacklin/src/m/node_modules/glob-watcher/node_modules/gaze/lib/gaze.js:129:32)
    at /Users/rmacklin/src/m/node_modules/glob-watcher/node_modules/gaze/lib/gaze.js:389:18
    at Array.forEach (<anonymous>)

which is caused by this line (on the current master branch, it's this line):

sass-graph/sass-graph.js

Line 117 in f6aa195

filepath = fs.realpathSync(filepath);

Since the watched file was removed, fs.realpathSync throws that error. However, the event.path value that gets passed for the filepath parameter is already an absolute path, so it seems like in that scenario it is unnecessary to call fs.realpathSync. Does that seem reasonable? If so, would it be okay to add a guard check like if (!path.isAbsolute(filepath)) before we call fs.realpathSync?

[rmacklin]

If so, would it be okay to add a guard check like if (!path.isAbsolute(filepath)) before we call fs.realpathSync?

I've opened a PR with this change: #100

[xzyfer]

The expected way to handle this to look at the event type. If it's a deletion event then remove the path from the index. That's how it's done it node-sass.

…

On Thu., 3 Jan. 2019, 11:37 am rmacklin ***@***.*** wrote: If so, would it be okay to add a guard check like if (!path.isAbsolute(filepath)) before we call fs.realpathSync? I've opened a PR with this change: #100 <#100> — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#99 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAjZWI01C9EUVz_j1-K9v-tuSW4fPr7Qks5u_VDMgaJpZM4ZnbFK> .

[rmacklin]

Thanks for your response!

If I remove the path from the index, it won't trigger recompilation of the ancestors. But I want that to happen so that the watch task will display an error if you remove a file that is still imported somewhere. If it's removed from the index, the watch task will just silently carry on despite there being broken code that the developer should address immediately.

Is there another solution you'd be open to?