No compiler errors in stderr

[tamagokun]

Using jaded-brunch to compile some static pages, and it doesn't seem to report any compiler errors.

Using version 1.7.11 and brunch 1.7.14. Compiling app/index.static.jade. I've tried setting:

plugins:
  jaded:
    jade:
      compileDebug: true

but still no luck.

[monokrome]

Thanks for the report, @tamagokun. Can you show me the rest of your configuration? Are jade files matched as templates in your configuration file?

I'll look into this ASAP. Let me know if you find anything else out.

[tamagokun]

Sure, here's my brunch-config:

exports.config =
  files:
    javascripts:
      joinTo:
        'javascripts/app.js': /^app/
        'javascripts/vendor.js': /^(vendor|bower_components)/
      order:
        before: ['bower_components/jquery/dist/jquery.js']

    stylesheets:
      joinTo: 'stylesheets/site.css'
      order:
        before: ['bower_components/*']

Here are my deps from package.json:

  "dependencies": {
    "auto-reload-brunch": "^1.7.3",
    "bower": "^1.3.8",
    "brunch": "^1.7.14",
    "clean-css-brunch": "^1.7.1",
    "coffee-script-brunch": "^1.8.0",
    "css-brunch": "^1.7.0",
    "jaded-brunch": "^1.7.11",
    "javascript-brunch": "^1.7.1",
    "stylus-brunch": "^1.8.1",
    "uglify-js-brunch": "^1.7.7"
  }

Files:

app/
---- index.static.jade
public/
---- index.html

If there are no jade compile errors and it compiles the template, things work great. The proper HTML is getting written to index.html.

I tried to do a simple test inside the compile function to just see if I could return an error into the callback, and even that didn't seem to log anything so I thought maybe this could be a bug with brunch and not this plugin, but I just can't pinpoint it.

[monokrome]

@tamagokun Thanks for reporting this. I can duplicate it here, and it's definitely seeming to be a brunch issue.

I've verified this by making sure that I am passing an error into brunch's callback, which I am doing here. The error from jade exists intact at this point during execution. I'd like to know if you can log the error yourself at this point to validate my local observation here. It seems like brunch is ignoring the error and just moving on, even though I have provided it to the callback.

This might be an oversight related to jaded requesting to take care of static file storage separately from brunch, but I'd hope that brunch will still handle error cases at times like this. Otherwise, how are we to maintain a consistent error reporting mechanism?

It's possible that jaded-brunch is doing the wrong thing here, but it seems like an API has definitely changed in brunch to cause this issue.

Pinging in @paulmillr to take note of this as well. I've also created an issue at https://github.com/brunch/brunch/issues/861 regarding this. I'm going to leave this open while the other issue is considered, and close it after brunch has solved the problem.

In the worst case, if brunch feels that it is a jaded-brunch-specific problem, then I will do what is necessary to get errors to be properly reported via other means.

[tamagokun]

Yeah, in the catch block when compiling the template if I do something like console.log(error.toString()) it prints out the jade error.

[monokrome]

@tamagokun The line that I linked is in the successHandler function. That is the point of code where the error is given back to brunch. Did you try there as well?

[tamagokun]

Yes tried it there as well. Using console.log printed the error properly, but sending it through to the brunch callback didn't log the error.

[monokrome]

Okay. It looks like this is definitely an issue in brunch. I think that we'll need to hear back from the brunch team on this one.

Thanks again for reporting the issue!

[paulmillr]

Why do other plugins report errors in this case?

[tamagokun]

Just for a sanity check, I even tried:

JadedBrunchPlugin.prototype.compile = function(data, path, callback) {
  return callback("Error", null);
}

and still nothing is getting logged. Is there a configuration setting not right?

[paulmillr]

DEBUG='brunch:*' brunch watch should help w debugging

[tamagokun]

File in question: app/index.static.jade

oooo pretty colors. Still no sign of anything. I'll keep digging.

[monokrome]

@paulmillr I mentioned this before, but just to confirm that you saw it - is it possible that this could be an oversight with how jaded has requested to store the file manually instead of resolving the storage process through brunch? I would expect brunch to still handle errors in this case, but maybe it isn't?

EDIT: Never mind. This occurs before jaded ever even tries to let brunch know that it wants to store things.

[tamagokun]

Changing the type to javascript rather than template seemed to allow the errors to flow. In fact, the plugin seems to still function just fine even though the type has changed.

[monokrome]

@tamagokun That's definitely interesting. It seems like it would a bit odd from a user perspective to put your template configurations under a javascripts configuration, though.

@paulmillr Is this expected behavior? If so, then why is it that templates are not allowed to throw errors?

[tamagokun]

@monokrome I would assume that this means the process of handling templates and scripts slightly differs.

[monokrome]

@tamagokun The type isn't used by the plugin itself, but just provides some form of (in my opinion, restrictive) identification for how the plugin should be configured. I personally stopped using brunch because I didn't like these types of arbitrary restrictions as well as that brunch seemingly completely ignores it's own dependency graphs.

You might run into that second issue with this jade plugin, because saving the super template when one extends the other doesn't actually mean that the other will rebuild even though jaded uses progeny to explain the dependencies to brunch. :(

The only difference for templates vs javascripts on brunch's side should be the strings used for concatenation as well as maybe a different implementation for source maps. Maybe I'm missing something here, but we'll see what @paulmillr says.

[monokrome]

@tamagokun No response from @paulmillr yet. Maybe we're going to need to work around this in some other way :(

[paulmillr]

there should be no difference between js and templates in this case.

[monokrome]

@paulmillr So, it's safe to say that this is a brunch bug then? We're giving the error to brunch, it works when the type is set to javascript but fails when it's template.

[paulmillr]

@es128 what do you think?

[es128]

I'll dig into it and see if I can find the underlying issue. Based on reading through this thread, it does seem like a bug in brunch.

[es128]

@tamagokun What does the jade file look like when it has an error?

I just tried throwing some garbage into a .static.jade file in a simple skeleton that uses this plugin, and here's what I got (took a few tries to actually cause an error, jade is pretty forgiving):

~/Code/brunch-static-skeleton
λ brunch w
07 Aug 16:41:21 - info: compiled 3 files and 2 cached into 2 files, copied 3 in 280ms
07 Aug 16:41:34 - info: compiled in 75ms
07 Aug 16:41:57 - info: compiled in 70ms
Warning: missing space before text for line 3 of jade file "app/blah.static.jade"
07 Aug 16:42:15 - info: compiled in 71ms
Warning: missing space before text for line 3 of jade file "app/blah.static.jade"
07 Aug 16:45:18 - info: compiled in 72ms
Warning: missing space before text for line 3 of jade file "app/blah.static.jade"
07 Aug 16:45:36 - info: compiled in 72ms
07 Aug 16:45:49 - error: Compiling of 'app/blah.static.jade' failed. app/blah.static.jade:3
    1| html
    2|  asdfl asdfkl
  > 3|   asdfkljh.asdf#asdf/adsf#..li n./asd/./.asdf/asdf/gjafsl;iuo88978y y8uo
    4|

Invalid indentation, you can use tabs or spaces but not both

Also, the error handler in the callback provided to the compiler is at https://github.com/brunch/brunch/blob/master/src/fs_utils/pipeline.coffee#L48 - doesn't seem like it does anything differently per type (script vs template).

[tamagokun]

huh. It seems to be working now. I did need to re-run npm install before I re-tested, so I wonder if any updates occurred since before.

@monokrome can you confirm?

[monokrome]

@tamagokun @es128 I'm still seeing the issue as originally explained. I'm seeing it with the code available at this repository which I've created: https://github.com/monokrome/jaded-test/tree/script-error

I've run rm -rf ~/.npm && npm install as well, in order to make sure that there aren't issues in my NPM cache and such.

[es128]

@monokrome Thanks for the test case.

I've found the problem. When a source file does not match any joinTo target, it disrupts brunch's control flow. Adding something like this in the jaded-test example's config fixes the issue:

    templates:
      joinTo: 'doesntmatter'

I'm thinking about a good way to solve this within brunch core so that the config hack isn't required in this type of case.

[monokrome]

@es128 Oh, that seems a bit more obvious now.

I was so focused on why brunch wasn't responding to the error case (which is probably still an issue) that I didn't even think about the fact that there aren't any templates in the configuration file...

It's only getting compiled because it matches the app.js, I think. However, that seems to even be weird considering app.js doesn't actually use the templates type. Why is this file even being compiled in this case?

Brunch is seemingly using a template plugin to compile into javascript output?

[es128]

Brunch is seemingly using a template plugin to compile into javascript output?

Yes, that's the whole reason your plugin exists afaict. Brunch has always assumed that template compilation would lead to concatenated javascript output (such as in jade-brunch). It's a long-standing limitation, half solved here by allowing plugins to return a null compilation result and take over writing the output file. Part of the filesets concept I proposed for Brunch a long time ago and unfortunately have not yet executed on is about solving this problem.

[es128]

It's only getting compiled because it matches the app.js, I think.

Actually, no. It's getting compiled because it has a file extension that matches one of the plugins. But then even after being compiled it gets abandoned because Brunch has nowhere to send it.

I'm actually finishing up a two-fer improvement as a result. 1) properly emit compiler errors as was originally expected; 2) print a warning whenever a compiled file with a non-empty result has no concatenation target, indicating a misconfigured joinTo.

That second scenario has led to confusion and been the underlying problem of multiple reported issues, so I think adding the explicit warning there will turn out to be pretty helpful.

[monokrome]

@es128 I think that you've missed what I'm wondering.

1) Since you're saying that it doesn't match the pattern for app.js, then why does Brunch compile something that isn't even matched by any of the patterns? What I'm suggesting is that there's no reason that this file should have been compiled in the first place, because it doesn't go anywhere. It's unnecessary processing unless brunch is asked to compile it via config.json, isn't it?

2) It doesn't get "abandoned". This plugin writes it to public/index.html manually. It tells brunch that it wants to manually manage storing the file by sending null to the callback. Hopefully your warning will account for this case as well? The real reason that this is an issue leads back to #1, however.

It's probably worth moving conversation over to the brunch issue since the issue is a brunch bug.

[es128]

1) You're correct in that it can turn out to be unnecessary processing (generally due to misconfiguration). That's why I think it makes sense to add a warning about it. But this behavior is also what allows jaded-brunch to do its static compilation - filtering based on joinTo matches earlier would prevent that.

2) I meant abandoned in terms of the remaining control flow. That's why the error wasn't being emitted. And the new warning will only be printed when the compiler does return data, but then Brunch has nowhere to send it. So use of jaded-brunch's static compilation feature will be unaffected.

[monokrome]

@es128 That's not how it seems to work from an interface perspective. I think that you may be confused?

Even if you have the file in a joinTo brunch will NOT join it with this plugin's static files, because the plugin has requested exclusively rights to store that file as it wants. Since that is the case, a joinTo is required because the configuration format is a mess - as I had previously discussed (and was ignored) during this discussion - you can now see where this format is broken first hand. You can also see that these problems are not "out of scope". The issues are specifically related to problems that the format that being recommended there is inspiring, but I digress.

These are the kinds of issues that I was trying to make Brunch aware of before the new file-sets stuff was around. The configuration is defined around brunch's internal concepts, instead of being a way to nicely expose them. Now that these issues exist, we need to work around them:

This means that you are required to use a joinTo because of the configuration format. However, when you do use one - it will not output into the joined file regardless.

Either way, there is a bug in brunch. So, this discussion is now not even about jaded-brunch and belongs here.

[es128]

not confused afaik :smile:

The file matching a joinTo does actually take effect and allow the remaining flow to continue. That's why the config hack above worked. There's no evidence of it in the output file however because the result of compilation by the plugin was no data.

[monokrome]

@es128 I don't know why GitHub posted my comment while I was still writing it, but it is complete now. I assume you started reading it before it was finished, because you replied nearly immediately after I hit submit.

It said "Posted 8 minutes ago" immediately after I submitted it as well, so GitHub is doing something weird.

[es128]

joinTo is not required for files whose output will be written by the compiler. There was a bug in compiler error handling for this case, but it's just been fixed.

[monokrome]

For the third time, please take it here: https://github.com/brunch/brunch/issues/861

[monokrome]

@tamagokun According to @es128, the issue is fixed. Thanks for reporting the issue!