Brunch doesn't let plugins take control of storage

[monokrome]

Brunch assumes that a plugin's compile() method needs to pass data to a callback. This is almost always correct, but there are rare cases (most commonly with templates) where a plugin might need to build static files or something else and they don't want to write something through brunch's callback.

In these cases, it would be nice if could pass a specific value as the second argument to the callback (maybe something like false) to tell brunch that no error has occured, but my plugin is going to handle storing this specific document instead of brunch.

I am wishing that I had this feature do to a recent project that I created because brunch/jade-brunch doesn't support static file rendering. I wrote my own which does. Here's an example of where I'd like to use this feature:

https://github.com/monokrome/jaded-brunch/blob/master/src/index.coffee#L110

[paulmillr]

what’s wrong with passing empty string today?

[monokrome]

Currently, my plugin is passing an empty string.

However, the problem with passing an empty string is that a module is still defined for the empty string. There shouldn't be unnecessary modules in app.js, and it seems like checking for empty string to see if a module should be added into app.js or not isn't the best way to check if a module should be added or not. Some compilers could generate an empty string on purpose, so you could run into issues in that case.

If I have a plugin which knows that a file specifically marked as static, that file should never result in a module in app.js. Not only does it include extra unnecessary code, but some people might want to have a file named index.coffee next to something named index.jade where they expect index.jade to be statically compiled. In the current case, this would cause a name collision and unexpected behavior could occur since require('index') might return the result as compiled by index.jade instead of the results from index.coffee.

Since the user expects static files in my use case, it should be realistic to suggest that unnecessary modules shouldn't need to be in app.js - especially with concern to the name collisions issue.

[paulmillr]

feel free to submit a pull req for this then

[monokrome]

@paulmillr I've looked into doing this twice now, but the flow of Brunch's code is a bit confusing to me. I'm not sure how the plugins are used; particularly where the actual callback provided to the Plugin#compile() method is defined.

Any tips that could help me implement this feature would be appreciated.

[paulmillr]

https://github.com/brunch/brunch/blob/master/src/fs_utils/source_file.coffee#L27-54

[monokrome]

I tried to implement this, but there is apparently some source map support that didn't know what to do in the case where the resulting file wasn't in the cache. I wasn't sure how the caching stuff worked, and don't have time to continue looking into it for a little while.

Thought that I'd communicate an update here.

[paulmillr]

fixed

[monokrome]

Awesome. Will this be part of the 1.7 update?

[paulmillr]

yup

[monokrome]

I was confused on the errors that came from the cache values when I did this. Good to know that I was on the right track before I stopped.

Thanks!

[paulmillr]

Can you please help us with testing of 1.7 for your plugin? I want to release it just after proper testing and update of screncast.

[monokrome]

I just published the v1.7.0 of my Jade plugin in case you have a version of brunch to try it against.

http://registry.npmjs.org/jaded-brunch

Are you asking me to write unit tests for this feature, to write unit tests for my plugin, or to just manually make sure that it is working as I expect? Not sure which one.

[paulmillr]

Just manually test this stuff, should be enough

[monokrome]

Sure enough, it works as expected. Here's my relevant code to make sure that it's doing what you expect on your end:

https://github.com/monokrome/jaded-brunch/blob/master/src/index.coffee#L112

[monokrome]

@paulmillr

I wanted to note that a recent change has potentially broken this in brunch. I expected that performing callback() without any arguments would let brunch know to move on without storing anything, but lately there has been an error in fs_utils/pipeline where it is checking if compiled using the ? operator. This has caused it to be considered 'compiled' when the actual output from the plugin is undefined.

I've fixed this in my plugin, but still think that it's worth mention and possibly creating a new issue if you think that it's not working as intended. Here's the change that fixed the issue in my plugin, but I feel like the old version was 'better':

monokrome/jaded-brunch@c610e2e#L2L115

[paulmillr]

? checks for == null which checks for undefined

[monokrome]

You are right, but the error still existed somehow.

[paulmillr]

I’m pretty sure it should work. I just checked coffee output. If it's really brunch, make sure to debug the bug, PRs are welcome, should be super simple here

[monokrome]

@paulmillr Please note that the issues that I am describing in the last comments of this issue are the same issue that has been discovered in #694.

[lalomartins]

Here via #916 — see also #1007

As far as I can see, in the time since, the module is no longer created if I pass an empty string (which I've done for jade-static).

[monokrome]

Thanks, @lalomartins. I'll give it a try in jaded-brunch :)

[lalomartins]

So no, I was wrong, empty modules are still created — I wasn't seeing them because my templates.joinTo was unset ;-)

That said, if you pass in none or undefined, dependencies aren't tracked, as discussed in #916, so for now it's still better to have the empty modules…

[monokrome]

@lalomartins I think that it's better to leave it as is for now instead of having unnecessary code running in production, but mostly because this issue should be solved on the brunch side.

[lalomartins]

Agree with the sentiment, and I'll probably start a branch soon (I've been looking at the pipeline sources already).

That said: in my project I'm using the forked version that returns '', and I dealt with the unnecessary generated code by joining it to a dummy.js file that never gets referenced by the html.

[monokrome]

@lalomartins That's a clever solution for use in the meantime! :)