Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keep watching files imported in sass also when they have errors #134

Closed

Conversation

endaaman
Copy link

If you build following codes

require('./style.scss');
// style.scss
@import "external";
// _external.scss
.red {
  color: red;
}

and change _external.scss with error such as syntax error like

// _external.scss (missing closing brace)
.red {
  color: red;

then Webpack will not recompile the scss files.

And If you build style.scss importing _external.scss missing closing brace firstly and modify _external.scss, Webpack will also not recompile them.

This PR fixes them.

@jorrit
Copy link
Contributor

jorrit commented Jul 31, 2015

There is a console.log(err) in the second commit. Also, can you squash the commits?

@endaaman
Copy link
Author

I'm very sorry. Im just an amateur at coding.
You don't have to merge this PR. But please acknowledge the problem...

@jorrit
Copy link
Contributor

jorrit commented Jul 31, 2015

Wouldn't it be easier to just do addIncludedFilesToWebpack([err.file]) in both error handlers or am I overlooking something ?

@endaaman
Copy link
Author

Do you mean it should be like this?

    // line: 165
    sass.render(opt, function onRender(err, result) {
        if (err) {
            addIncludedFilesToWebpack([err.file]);
            formatSassError(err);
            callback(err);
            return;
        }

If it is so, this does'nt work well.

// style.scss
@import "external";
//  _external.scss
@import "more-external";
// _more-external.scss (missing closing brace)
.red {
  color: red;

If you build this, fail and comment out @import "more-external"; in _external.scss, Webpack will not recompile because Webpack is watching only _more-external.scss and can't detect _external.scss is modified.

@jorrit
Copy link
Contributor

jorrit commented Jul 31, 2015

I now see what you mean. It would definitely be easier with squashed commits :) it is not very hard to do, see https://github.com/ginatrapani/todo.txt-android/wiki/Squash-All-Commits-Related-to-a-Single-Issue-into-a-Single-Commit

@jhnns
Copy link
Member

jhnns commented Aug 5, 2015

Thx for your effort @endaaman. 👍

The current master branch should fix your problem. However, I've used another approach because your PR adds files as dependency that might not exist anymore. This might be no problem, but I don't think that it's good coding style (no offence). But your PR was inspiration for the current solution 😀.

@endaaman
Copy link
Author

endaaman commented Aug 6, 2015

I just confirmed this problem is fixed! Thanks ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants