CSS isn't compiled after the first success?

[shawnbot]

I'm running into a weird issue using this middleware with Express. Here is our usage:

var sass = require('node-sass-middleware');
app.use('/css', sass({
  src: __dirname + '/style/sass',
  dest: __dirname + '/style/css',
  outputStyle: 'nested',
  debug: true,
  force: true,
  sourceMap: true
}));

Our issue is that if we don't pass force: true, the CSS for our source file only compiles successfully the first time, and 404s after that. I've cloned the repo and added a test case that attempts to simulate this by simply requesting a stylesheet twice in a row, but I can't reproduce it.

Here's what happens on the console:

1. The first request outputs the sass middleware's debug messages, as expected:

   source: /Users/allens/work/eiti-data/style/sass/main.scss
   dest: /Users/allens/work/eiti-data/style/css/main.css
   read: /Users/allens/work/eiti-data/style/css/main.css
   render: /Users/allens/work/eiti-data/style/sass/main.scss
   render: /Users/allens/work/eiti-data/style/css/main.css.map
   source: /Users/allens/work/eiti-data/style/sass/main.scss
   dest: /Users/allens/work/eiti-data/style/css/main.css
   

2. The second time around, nothing happens and Express returns a 404.

3. The third time around, the middleware outputs just the source and dest paths, but no render calls:

   source: /Users/allens/work/eiti-data/style/sass/main.scss
   dest: /Users/allens/work/eiti-data/style/css/main.css
   

I can try to create a standalone test case for this if you'd like, but I just wanted to make sure that I'm not doing something glaringly wrong or missing something totally obvious. As you can see, there's nothing very complex going on in our SCSS source. Thank you!

[am11]

Thanks for reporting the bug.
Are you using latest v0.9? It could be due to the newly added feature in 0.9.
If you are seeing this issue with 0.9, please test with 0.8 (npm install node-sass-middleware@0.8) and let us know if you find anything interesting. :)

[shawnbot]

Thanks for responding! I've downgraded to 0.8 and am still seeing the same issue.

[am11]

Thanks for the effort! :)
@Xiphe, @imoritz, could you guys provide some pointers on this? I will have a look over the weekend. In the meanwhile, if you could provide a fix for it, that would be brilliant! 🌟

[am11]

@shawnbot, sorry for being late.

As it turned out, this is not a bug its a (sleek) feature! 😎

If the content of Sass source has not changed since the last successful compilation, the middleware will prevent the recompilation. The condition compares the mtime (modified time) of CSS file with that of Sass.

With force:true, it disregards those checks and compile in all cases.

You can safely remove force: true and be assured that whenever you make the change while the server is up and running, the subsequent request will recompile (I have confirmed this by running the doi express server). This way you will be saving some significant per-call time/performance penalties.

Please feel free to reopen if you still believe there is a problem. Thanks.

[shawnbot]

@am11 Unfortunately, that's not what I'm seeing. The bug described above crops up when I remove force: true. Here are my steps to reproduce. First, clone and install the dependencies:

% git clone https://github.com/18F/doi-extractives-data.git
% cd doi-extractives-data
% npm install

Next edit server.js on line 57 and set force: false.

Then, start the server and request the stylesheet twice:

% npm start

% curl -I http://localhost:6002/css/main.css
HTTP/1.1 200 OK
X-Powered-By: Express
Content-Type: text/css
Cache-Control: max-age=0
Date: Mon, 22 Jun 2015 18:04:10 GMT
Connection: keep-alive

% curl -I http://localhost:6002/css/main.css
HTTP/1.1 404 Not Found
X-Powered-By: Express
X-Content-Type-Options: nosniff
Content-Type: text/html; charset=utf-8
Content-Length: 26
Date: Mon, 22 Jun 2015 18:04:12 GMT
Connection: keep-alive

At this point, I can see that style/css/main.css (the CSS rendered in the dest directory) is freshly touched. I'm on GMT-7, so the Date header in those responses matches up with the file's mtime:

% ls -la style/css/main.css
-rw-rw-r--  1 user  blah   90 Jun 22 11:04 main.css

The same thing happens when I rm the file before requesting it: it's re-rendered and served up properly the first time, then gets a 404 the second time.

[shawnbot]

Is it possible that none of the imports was changed and that it's just calling next() in this condition? That would produce a 404, right? If so, shouldn't it be serving up the already-compiled .css file instead?

[am11]

I have not worked with middleware architecture much (just used Owin couple of times), but the concept of middleware is to pass on to the next one, and hence the request flow should continue until handled; response is produced. Throwing exceptions or ending the flow happens usually in the finalizer middleware. next() shouldn't necessarily means that the error has encountered, but that the prev one did not handled the given request.

With that said, In your usage, you can probably do something like:

app.use('/css', sass({ 
   src: __dirname + '/styles/sass', 
   dest: __dirname + '/styles/css', 
   outputStyle: 'nested', 
   debug: true, 
   force: true, 
   sourceMap: true 
 }))
 .use(function(err, req, res, next) { 
    if(err) {
      res.statusCode = 500;
      return res.end(err.message);
    }

    var  destination = _dirname + '/styles/css' + require('url').parse(req.url).pathname;

    if(!fs.existsSync(destination )) {
      res.statusCode = 404;
      return res.end('Compilation failed: Destination file not found.');
    }

    res.writeHead(200, {
      'Content-Type': 'text/css',
      'Cache-Control': 'max-age=0'
    });
    res.end(fs.readFileSync(destination));
 }); 

If this does not satisfy the issue either, please reopen this issue and someone well familiar will investigate.

[ohmyhusky]

Hi there, I've got the same 404 res just like what shawnbot got, Looks like if we don't add "force:true", it won't recompile , and won't sent to client either.

[am11]

Hello @cyl19910101, see the related discussion at: #54.

[ohmyhusky]

Thank you very much for responding @am11 , this is kind of a different circumstance from #54, And I figure out why shawnbot and I got the same 404 error while other guys just worked well. That's because we didn't add an extra static file middle ware to read the already exist css file which will be invoke in your code: " changed && changed.length ? compile() : next(); " --We didn't add the next middle ware.
I think maybe the reason that me and some other guys write code like this(didn't add a static file handler) is misunderstood your code sample :
app.use(sassMiddleware({
/* Options */
src: __dirname,
dest: path.join(__dirname, 'public'),
debug: true,
outputStyle: 'compressed',
prefix: '/prefix' // Where prefix is at
}));
app.use(express.static(path.join(__dirname, 'public')));
the last line add a middle ware to handle the static file, that's looks like a default express temple code so we didn't realise that this code is used to handle the already exist css file. However, in the Connect example you gave us, we can easily find out the connection between node-sass-middleware and static file middleware by the str '/prefix'. I don't use connect, so I didn't realise that's what I should do. Sorry about that~
BTW, in your code, if we need to compile sass file, you use res to send css back to client:
res.writeHead(200, {
'Content-Type' : 'text/css',
'Cache-Control': 'max-age=0'
});
res.end(data);
And that't the reason why we can got it first. And when I said " it won't recompile , and won't sent to client either. ", I kind of express my circumstance in a wrong way, what I really mean is , it won't recompile -- that's what we want, but it won't send to client -- that's the problem.
Finally, I figured it out that this is my mistake, so sorry about that, and I very appreciate your work, thank you very much!

And for some other user may have the same problem like me, Might I suggest you that use the same way to handle the response that both send back in node-sass-middleware whether it need to compile, or both use an extra static file middle ware to handle the response? If so, they won't get the css file unless they use a correct static file middle ware setting, Or they can get the css file whether they use a static file middle ware. Maybe simply change the express sample will do too.

I'm not good at english, sorry to let you read a post like this~

[am11]

@cyl19910101, thanks for spotting the issue and for the working solution. We should highlight this information in Readme, so in future, other users don't get confused. PR is welcome! :)

Regarding using the static file middleware instead of invoking next() blindly; I don't have strong opinion about it but I would like to share a thought:

While this is the most common usecase that users would like to have static css file handler put as a next to node-sass-middleware, but IMO it goes against the middleware architecture itself. The idea of middleware is to decide whether you want to handle certain request, if so handle it and return to the next middleware, otherwise return to the next as is immediately, without handling it. Now if we provide an option to override this behavior and chose the next static file middleware, this way node-sass-middleware will act like an API which changes the control behavior based on the option, instead of being a lightweight middleware -- exercising the principle separation of concerns. Note that the existing options are mainly passed on to node-sass upstream, there are some exceptions.

If you still think this would be a good idea to have another exceptional option like use static handler as a next middleware, then please open a new issue to make it visible for others and we can gather more opinions.

[torfsen]

I'm having the same issue as @shawnbot: Upon the first request, the .sass file is compiled and the resulting .css file is served correctly. All following requests only return 404.

Here's my express setup:

app.use(sassMiddleware({
    src: path.join(__dirname, 'views'),
    dest: path.join(__dirname, 'public'),
    indentedSyntax: true,
    prefix: '/public',
    debug: true,
    outputStyle: 'expanded'
}))

app.use(express.static(path.join(__dirname, 'public')))

First request:

$ rm -f public/style.css 
# At this point the server is started
$ curl http://localhost:3000/public/style.css
h1 {
  color: red;
}
$ ls public/style.css 
public/style.css

Corresponding output on the server log:

  source: /home/torf/projects/muell-wecker/views/style.sass
  dest: /home/torf/projects/muell-wecker/public/style.css
  read: /home/torf/projects/muell-wecker/public/style.css
  render: /home/torf/projects/muell-wecker/views/style.sass

Next request:

$ curl http://localhost:3000/public/style.css
Cannot GET /public/style.css

Corresponding server log:

  source: /home/torf/projects/muell-wecker/views/style.sass
  dest: /home/torf/projects/muell-wecker/public/style.css

Removing the compiled .css file fixes the problem for one request:

$ rm public/style.css 
$ curl http://localhost:3000/public/style.css
h1 {
  color: red;
}
$ curl http://localhost:3000/public/style.css
Cannot GET /public/style.css

Similarly, restarting the server (without removing a previously compiled .css file) again only works for one request:

# Restart server here
$ ls public/style.css 
public/style.css
$ curl http://localhost:3000/public/style.css
h1 {
  color: red;
}
$ curl http://localhost:3000/public/style.css
Cannot GET /public/style.css

If I add force: yes to my options then it works as expected:

$ curl http://localhost:3000/public/style.css
h1 {
  color: red;
}
$ curl http://localhost:3000/public/style.css
h1 {
  color: red;
}

However, I don't think force: true should be required for this to work.

There's a similar (unanswered) question on StackOverflow regarding this behavior.

I'm using node-sass-middleware 0.10.0 with express 4.14.0.

[am11]

@torfsen, does it work if you replace:

app.use(express.static(path.join(__dirname, 'public')))

with:

app.use('/public', express.static(path.join(__dirname, 'public')))

See a working example at: #70 (comment) and working project at #70 (comment).

[torfsen]

@am11 That seems indeed to fix the issue, thanks!

I copied my original code from the Express example in the README, so I've created a PR (#89) to fix that example.