updating included files even in case of compilation error (Lint Mode)

[akiran]

I previously opened this issue in libsass.
sass/libsass#638

@mgreter made changes to API in libsass.
When can we expect this in node-sass ?

[am11]

We have another level of refactoring going on. After all the other v2.1 dust is settled, we will add this feature.

Some notes for future:

• We will probably add it as a "lint mode", with existing output format or a different.
• For lint mode, we can actually accept user-defined custom reporters (like jshint, jscs, tslint, coffeelint etc.).

I am adding it to unknown milestone for now.

Thanks for reporting this.

[akiran]

@am11 Thanks

We needed this feature in sass-loader to support watch functionality (#528)
Will it be possible to pass stats to error handler similar to success handler ?

@jhnns Any comments ?

[xzyfer]

We needed this feature in sass-loader to support watch functionality

I may not be fully understanding the issue. @akiran is it possible to just use sass-graph for this? This is how node-sass' smart watcher works #629.

[akiran]

@xzyfer , We are already using sass-graph in sass-loader to support watchers. But we are facing some problems.
webpack-contrib/sass-loader#50

We can avoid sass-graph dependency if stats are available to error handler.
As of now we are reading sass files twice, once for node-sass compilation and again to get dependency graph with sass-graph. This seems redundant.

[am11]

@akiran, technically it is possible to implement it so the error object is imploded with stats in error cb, but then we would have to make another call to collect stats in case of error; which is one reason it being a bad design. So IMO having the linter separated would be better. That will enable you to:

1- lint before compile if needed.
2- compile and get result or standard error if needed.

This will cause no impact on performance for user, who did not wanted stats / diagnostic information.

[akiran]

@am11, Yes, that will be great.

In case of sass-loader, we need list of includedFiles even in case of error to register dependencies with watcher. It is ok for us to call a different API function to get this info before compile step.

[am11]

The other approach is to make it disableable option.

Two reasons not to do so:

1- Too many (micro) options mean sloppy architecture.
2- Because we cannot guarantee that if the option is enabled, the error callback will always get the stats. One reason could be if the error occurs at parse time before / during libsass can illicit the info. So the spec will read like:

if verbosityLevel:Diagnostic (or whatever option) is set, you may or may not get stats.

Hence the non-persistent behavior.

Compare to:

if there is error during the lint, the linter will return back code 1 and a canned error message.

[jhnns]

Does it mean that linting is a separate function? From our point of view it does not make sense to parse the files two times, one time for linting and the other time for compiling.

In that case we'd probably lint the files after an error occurred to extract the included files.

[am11]

@jhnns, yes because libsass has it as a separate function and IMO we should not merge both in a single API call.

[jhnns]

Yeah, you're right. From our POV it would be ideally to receive the includedFiles in any case, but it's ok to just run the linting.