sass / node-sass

:rainbow: Node.js bindings to libsass
https://npmjs.org/package/node-sass
MIT License
8.51k stars 1.32k forks source link

Double notification on file change: body{background:blue}\nbody{background:blue} #1152

Open saper opened 9 years ago

saper commented 9 years ago

This is a follow up to #1150 which still does not fix the issues.

Our two tests (quoted below) sometimes (rarely) produce two lines of output. This looks like a double notification coming, from whatever reason. Maybe there is an interesting relation because we have both index.scss and index.sass in the directory.

I am going to mark those tests as flaky to avoid CI issues until we fix that.

  1) cli node-sass in.scss should watch the full scss dep tree for a single file (scss):

      Uncaught AssertionError: 'body{background:blue}\n\nbody{background:blue}' == 'body{background:blue}'
      + expected - actual

      -body{background:blue}
      -
       body{background:blue}

      at Socket.<anonymous> (test/cli.js:317:16)
      at readableAddChunk (_stream_readable.js:146:16)
      at Socket.Readable.push (_stream_readable.js:110:10)
      at Pipe.onread (net.js:523:20)

Test code:

    it('should watch the full scss dep tree for a single file (scss)', function(done) {
      var src = fixture('watching/index.scss');
      var foo = fixture('watching/white.scss');

      fs.writeFileSync(foo, '');

      var bin = spawn(cli, [
        '--output-style', 'compressed',
        '--watch', src
      ]);

      bin.stdout.setEncoding('utf8');
      bin.stdout.once('data', function(data) {
        assert.equal(data.trim(), 'body{background:blue}');
        bin.kill();
        done();
      });

      setTimeout(function() {
        fs.appendFileSync(foo, 'body{background:blue}\n');
      }, 500);
    });

    it('should watch the full sass dep tree for a single file (sass)', function(done) {
      var src = fixture('watching/index.sass');
      var foo = fixture('watching/bar.sass');

      fs.writeFileSync(foo, '');

      var bin = spawn(cli, [
        '--output-style', 'compressed',
        '--watch', src
      ]);

      bin.stdout.setEncoding('utf8');
      bin.stdout.once('data', function(data) {
        assert.equal(data.trim(), 'body{background:red}');
        bin.kill();
        done();
      });

      setTimeout(function() {
        fs.appendFileSync(foo, 'body\n\tbackground: red\n');
      }, 500);
    });
saper commented 9 years ago

It takes around of 15 minutes of running a pared-down version of tests/cli.js in a loop to reproduce those as above:

time sh -c 'while node_modules/.bin/mocha test/xcli.js ; do true; done; '
xzyfer commented 9 years ago

This could be a sass-graph issue. I'll take a look.

saper commented 9 years ago

I also feel there's something wrong in

https://github.com/sass/node-sass/blob/f6b5fd2de527a4cba599c6b38797a227463decb2/bin/node-sass#L247-L258

I'd love to have this moved to lib/watch.js and be properly tested internally.

saper commented 5 years ago

This needs to be tested again with the current code.

saper commented 5 years ago

Unfortunately, I was getting the same on 4.12.0 trying to debug https://github.com/sass/node-sass/issues/2755