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

Should auto included javascript controller/view files be minifed? #139

Open
tmazur opened this issue Oct 30, 2012 · 11 comments
Open

Should auto included javascript controller/view files be minifed? #139

tmazur opened this issue Oct 30, 2012 · 11 comments

Comments

@tmazur
Copy link

tmazur commented Oct 30, 2012

I expected auto included js files to be minified according to the filters set up in asset_compress.ini. But the files that are included are not processed in any way. After looking at the code in AssetCompressHelper.php this is the intended behavior, but stands in contrast with the v0.5 changelog "Controller/view javascript files are now combined into minified asset automatically now.".

Am I missing something? Is there any way to automatically minify these files?

@markstory
Copy link
Owner

They aren't automatically minified as that would require generating build file definitions for them or using the dynamic build files, which are not supported in the standard non debug deployment case.

@tmazur
Copy link
Author

tmazur commented Oct 31, 2012

What I'm looking for is simply minifying those files. I was thinking about adding a setting in asset_compress.ini 'filter_autoincluded'. If this would be set to true, running a shell build command would search for .js files in /js/views/*, run them through the filters, and finally save the filtered version to the cache path(preserving the directory structure).

So for example for an unfiltered file /js/views/posts/index.js we would end up with a filtered version of that file in /cache_js/views/posts/index.js. The helper would check the setting, check if the minified version exists, if not fall back to original. Simple and it works. Would you be willing to pull in such a feature? Maybe you have a better idea?

@markstory
Copy link
Owner

That seems like a reasonable way to do things. Having the build shell walk those directories and minify each file into a mirrored directory structure in the cachePath will be simple and makes sense to me.

@sebastienbarre
Copy link

Hi. Was wondering if there was any news on this one, that's a feature I would be very interested in (especially if it could also support the $timestamp options).
Thanks

@markstory
Copy link
Owner

There hasn't been any change here. I don't personally use this feature. Getting the autoincluded scripts working with the filter system might be a bit awkward. If you would like to take a stab at it, please do.

@sebastienbarre
Copy link

OK, thanks. I'll revisit this issue later, already took me a while to backport some of the recent changes on top of v0.5, we are still using CakePHP 1.3 here :) Thanks for this plugin, really helps.

@markstory
Copy link
Owner

Welcome :) I think the dynamic build files needs to be rethought in general. I find having two ways to create builds is awkward but I've not yet come up with a good way to unify them. If you have any thoughts on how the dynamic build files could be improved let me know.

@dereuromark
Copy link
Contributor

@sebastienbarre
Are you still interested in that feature? Maybe a PR with your proposed changes would speed things up and gets it merged in quicker.

@sebastienbarre
Copy link

@dereuromark I still am, though the changes were proposed by @tmazur

@dereuromark
Copy link
Contributor

Ops ;) Well then the PR advice goes to him then.

@tmazur
Copy link
Author

tmazur commented Jul 17, 2014

After looking into how this feature might be implemented, the required time overwhelmed me, and I put this on my 'todo some day far away' list. Anyway, if I find some spare time I will implement this feature and create a PR.

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

No branches or pull requests

4 participants