Respect the combine flag when dumping leafs #169

Closed
wants to merge 2 commits into from

2 participants

@mbence

Dumping assets in the dev environment (with the --watch switch) can take a long time. Especially if we use many files and some kind of a compiler (eg. Coffeescript). One solution is using the combine=true switch at the asset declaration. But the bundle ignores this setting, and still generates all leaf assets, even if they are never used. It is probably ok for debug purposes, but still takes a long time, and if you use some auto-update solution like https://github.com/mbence/LivePHPBundle it can be outright annoying (refreshing the browser multiple times while all the assets are generated).

I added a small change to the dumpAsset function, to watch for the combine setting, and if on, skip the leaf files. If you need to debug, you switch off combine, but if you want speedy development, then don't dump the unnecessary files.

I hope this would be useful for others too.
Thanks: Bence

@stof stof commented on an outdated diff Feb 15, 2013
Command/DumpCommand.php
@@ -186,8 +186,11 @@ private function dumpAsset($name, OutputInterface $output)
// start by dumping the main asset
$this->doDump($asset, $output);
- // dump each leaf if debug
- if (isset($formula[2]['debug']) ? $formula[2]['debug'] : $this->am->isDebug()) {
+ $debug = isset($formula[2]['debug']) ? $formula[2]['debug'] : $this->am->isDebug();
+ $combine = isset($formula[2]['combine']) ? $formula[2]['combine'] : !$debug;
+
+ // dump each leaf if debug but no combine
+ if ($debug && !$combine) {
@stof
Symfony member
stof added a note Feb 15, 2013

actually, this should be !$combine only, to honor combine=false in non-debug mode

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@mbence

I'm sure you all are very well aware of all this below, I only write this down to make it clear for myself. :)

To my understanding, the combine switch is only used to instruct Assetic to ignore the debug mode.
In debug mode (and in the dev environment) the leaf assets will be generated, and linked to. In prod and non-debug, only the combined main asset is generated and used.
If you want no debug (no leaf assets) in dev env you need to set combine=true or debug=false (<- surprise, an undocumented feature, but it's working as expected!)

The only problem is, that the --watch works only in debug mode, with --no-debug you get an error. So your only option is to add the combine=true if you want to speed up asset generation during development, and that was exactly my aim with this patch.

Using combine=false with --no-debug would be a completely new scenario, which makes little sense for me. If you want to debug (ie. have leaf assets generated), you would use the debug switch, wouldn't you? :)

@stof
Symfony member

@mbence The debug switch and the combine switch have different meaning. The goal of the debug mode is mainly to control if the cache should check for changes in the source or use the available cached file as soon as it exists to improve performance (which explains why --watch cannot work in non-debug mode). The debug flag also allows to skip some filters (for instance the minification of the JS file by using ?uglifyjs2 to apply the filter).
Combining assets together controlled by the combine flag. The only relation between the asset debug flag (defaulting to the Assetic debug mode which itself defaults to the symfony debug mode) and the asset combine flag is the default value of the combine flag.

@mbence

@stof Ok I understand now. I changed the line as you suggested.

@mbence

The original functionality is a little expensive, but pretty much fool proof. It will generate everything no matter what you set in your templates. Even if you change your templates your assets would still load, without a new dump. With this change you would have to run the dump again, after you changed the templates. But you will save a lot of time (or electricity, rain forests, or whatever, :))) - by not generating unnecessary files.

I think it's worth it.

@stof stof changed the title from Dumping leaf assets only if combine=true is not set to Respect the combine flag when dumping leafs Oct 14, 2014
@stof
Symfony member

Thanks for fixing this bug @mbence.

@stof stof added a commit that referenced this pull request Oct 14, 2014
@stof stof bug #169 Dumping leaf assets only if combine=true is not set (mbence)
This PR was squashed before being merged into the 2.4-dev branch (closes #169).

Discussion
----------

Dumping leaf assets only if combine=true is not set

Dumping assets in the dev environment (with the --watch switch) can take a long time. Especially if we use many files and some kind of a compiler (eg. Coffeescript). One solution is using the combine=true switch at the asset declaration. But the bundle ignores this setting, and still generates all leaf assets, even if they are never used. It is probably ok for debug purposes, but still takes a long time, and if you use some auto-update solution like https://github.com/mbence/LivePHPBundle it can be outright annoying (refreshing the browser multiple times while all the assets are generated).

I added a small change to the dumpAsset function, to watch for the combine setting, and if on, skip the leaf files. If you need to debug, you switch off combine, but if you want speedy development, then don't dump the unnecessary files.

I hope this would be useful for others too.
Thanks: Bence

Commits
-------

e3cbd2f Dumping leaf assets only if combine=true is not set
f23d51c
@stof stof added a commit that closed this pull request Oct 14, 2014
@stof stof bug #169 Dumping leaf assets only if combine=true is not set (mbence)
This PR was squashed before being merged into the 2.4-dev branch (closes #169).

Discussion
----------

Dumping leaf assets only if combine=true is not set

Dumping assets in the dev environment (with the --watch switch) can take a long time. Especially if we use many files and some kind of a compiler (eg. Coffeescript). One solution is using the combine=true switch at the asset declaration. But the bundle ignores this setting, and still generates all leaf assets, even if they are never used. It is probably ok for debug purposes, but still takes a long time, and if you use some auto-update solution like https://github.com/mbence/LivePHPBundle it can be outright annoying (refreshing the browser multiple times while all the assets are generated).

I added a small change to the dumpAsset function, to watch for the combine setting, and if on, skip the leaf files. If you need to debug, you switch off combine, but if you want speedy development, then don't dump the unnecessary files.

I hope this would be useful for others too.
Thanks: Bence

Commits
-------

e3cbd2f Dumping leaf assets only if combine=true is not set
f23d51c
@stof stof closed this in f23d51c Oct 14, 2014
@mbence

Thanks @stof, I'm glad, that this fix made it in the master!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment