#5160 closed enhancement (fixed)
Use gruntjs to generate minified versions of the js and css files everywhere.
Reported by: | enej | Owned by: | johnjamesjacoby |
---|---|---|---|
Milestone: | 2.1 | Priority: | normal |
Severity: | normal | Version: | 2.0 |
Component: | Core | Keywords: | close |
Cc: | stephen@…, aaron@… |
Description
Since WordPress Core has adopted the use of grunt.
http://make.wordpress.org/core/2013/08/06/a-new-frontier-for-core-development/
It might be a good idea to follow suite. I would be willing to contribute a patch and instructions on how to do it.
Attachments (10)
Change History (75)
#2
@
11 years ago
- Milestone changed from Future Release to 2.0
Let's use this ticket to talk about adding Grunt to BuddyPress. In addition to just minifying CSS and JS, Grunt would let us have a better development workflow by better organising plugin code vs. any other bits of code that we currently have in trunk (e.g. the travis file), as well as opening the door to things in future like using a CSS pre-compiler. Having a convenient script to build a release package would also help the core team out when we do releases, and gives a couple of interesting possibilities for managing the deprecation of BP-Default and old bbPress.
If we did decide to utilise Grunt, the next issue is creating the Grunt file and deciding how we want the project organised. For example, WordPress core https://core.trac.wordpress.org/browser/trunk lives inside a /src/
subfolder, and the unit tests folder, some i18n tools, and a bunch of non-plugin config files live at the top level. I see this as a fairly neat way of organising the project's files, and I suspect we'd do something similar if we ever worked on a new plugin.
If we used a similar arrangement, we'd break any sites that do a SVN checkout of BuddyPress trunk; how many sites might be affected is unknown. We could assume that if you're running a SVN checkout of trunk, that you're keeping an eye on core development and would know how to fix the problems.
Another approach is not re-organising trunk, and just adding the Grunt file into the root /trunk/
folder. Biggest downside is that we mix build/test scripts with the actual code, and everything continues to get a bit messier.
I have put together proof-of-concepts for each idea:
- https://github.com/paulgibbs/bp-grunt (move trunk)
- https://github.com/paulgibbs/bp-grunt-lite (don't move trunk)
bp-grunt-lite
's Grunt build file is a little bit more fleshed out and tested than bp-grunt
is, but both should at least demonstrate the basic idea and generate a release build.
#3
follow-up:
↓ 4
@
11 years ago
DJPaul - Thanks so much for working on these. The implementation looks great.
I have some questions/thoughts about workflow.
- If we're going to go with bp-grunt-lite out of consideration for sites that are running trunk, then we should probably also add some more items to the 'dev' task. In particular, we probably need to add the bbPress external and cssmin for sites that are running trunk and also using these files.
- If we go with bp-grunt, and move trunk around, we could talk to the .org team about getting our own version their own split setup. bpdevelop.svn.wordpress.org which would then sync to buddypress.svn.wordpress.org. In that case, we might need another task for syncing to buddypress.svn, which would keep the tests files, run cssmin, run the bbPress external, etc. (I think this is a much more elegant solution, but it depends on nacin being able to help us with it.)
- Does anyone know how WP core Grunt builds integrate into their core dev workflow? I assume that there is a post-commit hook that runs the necessary build task on the server? Or do the devs do it locally?
#4
in reply to:
↑ 3
@
11 years ago
Replying to boonebgorges:
If we're going to go with bp-grunt-lite out of consideration for sites that are running trunk, then we should probably also add some more items to the 'dev' task. In particular, we probably need to add the bbPress external and cssmin for sites that are running trunk and also using these files.
For lite, I made an assumption that we'd remove the bbPress SVN external, and add in the bbPress files when building a release package as a way of maintain backpat yet keeping a dev environment clean as of much deprecated stuff as possible. For example, I'd propose to include BP-Default in the same way if/as/when we move it to wp.org/themes/.
Did you have further thoughts to other items for the dev build for lite? PHPUnit? We'd definitely want a watch
command added separately so we can lint JS during development. It looks like I didn't write this above, but with both the proposed bp-grunt and bp-grunt-lite, we'd no longer be committing minified CSS/JS to trunk. We'd only want those for the release packages. Sites running new /trunk/ as a checkout would load up the un-minified CSS/JS.
#5
@
11 years ago
with both the proposed bp-grunt and bp-grunt-lite, we'd no longer be committing minified CSS/JS to trunk.
Yup, I noticed this. But I think that if we go with lite, we need to commit them to trunk, for sites currently running trunk and not using SCRIPT_DEBUG.
I think it's a cool idea to run phpunit for the dev task, but I guess the feasibility of that may depend in part on whether it's getting run on our .org server - it'd need to be configured to have a test database, etc.
Aside from the concern about SCRIPT_DEBUG, I don't have any thoughts about other items to add to lite-dev.
#6
@
11 years ago
- Milestone changed from 2.0 to 2.1
We're going to grunt-ify BuddyPress trunk after we've shipped 2.0, so I'm moving it to the 2.1 milestone to keep the current release's milestone tidy.
#7
follow-up:
↓ 8
@
11 years ago
Does anyone know how WP core Grunt builds integrate into their core dev workflow? I assume that there is a post-commit hook that runs the necessary build task on the server? Or do the devs do it locally?
Do we have to be aware of anything in this respect? Currently I keep around half a dozen dev sites running bp checked out and kept updated from svn trunk does that now become files from the build branch? If so or even if not do we have to update our checkouts, change procedures in any manner, equally if creating a patch from our locals is there any change in the manner in which we would do that?
#8
in reply to:
↑ 7
@
11 years ago
- Cc stephen@… added
Replying to hnla:
Do we have to be aware of anything in this respect? Currently I keep around half a dozen dev sites running bp checked out and kept updated from svn trunk does that now become files from the build branch?
Per Paul's post here:
"A plugin loader file will be added to the root to prevent these changes breaking existing sites running trunk"
With bbPress we have a `bbpress.php` file in the root of the repo that if a /build
folder is found load bbPress from that folder, if not load bbPress from the /src
folder.
If so or even if not do we have to update our checkouts, change procedures in any manner,
Simply update the any of your SVN repos as you do now, no change is needed.
equally if creating a patch from our locals is there any change in the manner in which we would do that?
Again no change is specificly needed, ideally though when creating patches from the root of the repo your patches will now include the /src
path in the patch. Existing patches without the /src
in the path can be applied in the /src
folder and 'should' work fine in most cases.
Outside of the above having some environment variables in your profile as outlined in the BuddyPress Automated Testing doc will be helpful to to run BP Unit Tests in finding WordPress' unit tests.
You need to install Node.js
(If you have homebrew package manager, you can install Node.js with one command: brew install node
.)
Finally you need to install Grunt-CLI npm install -g grunt-cli
#9
follow-up:
↓ 10
@
11 years ago
Thanks, netweb. Most of this looks right, except:
Simply update the any of your SVN repos as you do now, no change is needed.
This is not entirely true.
- If your sites running trunk use bbpress 1.x (the legacy version packaged with BP) you will need to get it yourself, as it will not be included in the
/src/
directory. Egsvn co http://bbpress.svn.wordpress.org/tags/1.2 src/bp-forums/bbpress
. - If your sites running trunk do not use
SCRIPT_DEBUG
to run unminified versions of CSS and JS assets, you will either have to enableSCRIPT_DEBUG
or run your own local routine to minimize as necessary.
#10
in reply to:
↑ 9
@
11 years ago
- Cc aaron@… added
Replying to boonebgorges:
- If your sites running trunk do not use
SCRIPT_DEBUG
to run unminified versions of CSS and JS assets, you will either have to enableSCRIPT_DEBUG
or run your own local routine to minimize as necessary.
This can be coded around. We don't need to place the burden of SCRIPT_DEBUG on the developers running from src.
#11
follow-up:
↓ 15
@
11 years ago
This can be coded around. We don't need to place the burden of SCRIPT_DEBUG on the developers running from src.
I'd be curious to hear your thoughts on how this should be coded around. I see that WP is doing this: https://core.trac.wordpress.org/browser/tags/3.9/src/wp-includes/script-loader.php#L53 But we don't have the same luxury, because we use the same constant SCRIPT_DEBUG
, and it's already been defined by the time WP is loaded. In any case, we want to support cases where BP /src/ is run out of either /build or /src WP, so we wouldn't want BP to hijack the constant even if it could.
So, we could wrap SCRIPT_DEBUG
in a function like bp_script_debug()
, and in that function do a similar /src check to see if we should accept the value of SCRIPT_DEBUG
or whatever. Then we'd have to swap out every current call to SCRIPT_DEBUG
to use the new function.
I'm fine with doing that, but I will note that there are very few people running trunk in production anyway, and I would assume that they know what they're doing. We're a different beast from WP in this way.
#12
@
11 years ago
@netweb Thanks for the explanation, guess I'm not familiar with these tools so worry it might make make the ease of keeping a trunk checkout updated and in being able to rapidly patch when issues found and return those up, so in fact /src/ becomes the present trunk files we are checking out from svn is there any reason we don't simply change checkout path to reflect that and continue as normal as it were, in other words not be overly worried by build dir or tools?
#13
@
11 years ago
@hnla -- the build
dir won't be stored in version control. It only exists locally when/if you run the appropriate Grunt command. Take a look at bbPress trunk to get a better understanding of the layout.
The release/production versions of BuddyPress that we distribute on wordpress.org/plugins/ will basically be the contents of that build folder when we run Grunt.
#14
@
11 years ago
@DjPaul ah ok that makes sense.
Take a look at bbPress trunk to get a better understanding of the layout.
Yep just did and that helps clarify things.
Thanks Paul.
#15
in reply to:
↑ 11
;
follow-up:
↓ 45
@
11 years ago
Replying to boonebgorges:
This can be coded around. We don't need to place the burden of SCRIPT_DEBUG on the developers running from src.
I'd be curious to hear your thoughts on how this should be coded around. I see that WP is doing this: https://core.trac.wordpress.org/browser/tags/3.9/src/wp-includes/script-loader.php#L53 But we don't have the same luxury, because we use the same constant
SCRIPT_DEBUG
, and it's already been defined by the time WP is loaded.
bbPress is also using SCRIPT_DEBUG
, if it's defined in wp-config.php
then don't load (src) the minified CSS or JS
In any case, we want to support cases where BP /src/ is run out of either /build or /src WP, so we wouldn't want BP to hijack the constant even if it could.
Running WordPress from the the 'develop' repo's /src
folder will cause various issues, for example the 'MP6 admin colors', in the /src
folder there is only the .scss
files, there is no .css
files.
Further we have taken bbPress down this same route, our included 'MP6 admin colors' scheme does the exact same thing, we only have the .scss
files in the /src
folder then compile the .css
files as part of the build process to the /build
folder.
So, we could wrap
SCRIPT_DEBUG
in a function likebp_script_debug()
, and in that function do a similar /src check to see if we should accept the value ofSCRIPT_DEBUG
or whatever. Then we'd have to swap out every current call toSCRIPT_DEBUG
to use the new function.
I don't think this is needed, essentially if you are developing for WordPress core, BuddyPress or bbPress you should be running each of these using the /build
folder of each and patch against the /src
folder.
The grunt watch
task makes this workable as any changed files in the /src
folder when modified will be copied to the /build
folder with any extra tasks such as RTL'ing or minifying CSS will also occur depending on the file type.
I'm fine with doing that, but I will note that there are very few people running trunk in production anyway, and I would assume that they know what they're doing. We're a different beast from WP in this way.
The bbPress 'loader' is a 'workaround' for people who are running /trunk
and who have not run the 'build' task, it will load bbPress from the /src
folder so the site does not 'break' because bbPress /trunk cannot load. They will have issues with not having any RTL CSS, minified LTR/RTL CSS or JS and no MP6 admin color schemes.
#16
@
11 years ago
FTR, I've started writing the Gruntfile locally in preparation for this. I'll put it up on this ticket when done, and I suspect we'll start Gruntifying trunk after 2.0.1 ships and is stable.
#43
@
11 years ago
Awesome so far... Here are a few tweaks:
In 5160-php56.diff
- Only test PHP
5.6
beta with WP_VERSIONmaster
and allow to fail
In 5160-gruntfile-jshint-docs.diff
- Improved docs for Grunt task
grunt jshint
via [WP28222]
In 5160-rename-build-task.diff
- Rename Grunt task
grunt build-dev
togrunt build
- (Consistent Grunt task names across WordPress, bbPress and BuddyPress projects)
#44
@
11 years ago
In 5160-bump-bp-version.diff
- Bump BP Version in
package.json
to2.1.0
In 5160-watch.diff
- Update
grunt watch
task to dynamically clean and copy modified files from/src
to/build
- Automatically updates RTL and minified CSS and JS
- Fixes
grunt uglify
task to copy JS files from SOURCE_DIR - Adds
grunt uglify:dynamic
task for use withgrunt watch
- Adds
uglify
listener forgrunt watch
andgrunt watch:js
tasks
In 5160-makepot.diff
- Remove
buddypress.pot
from the repo - Remove
/src/bp-languages
from the repo buddypress.pot
is now generated by any of the grund build tasks in/build/bp-languages
grunt
,grunt build
,grunt build-commit
orgrunt build-release
- Headers in
buddypress.pot
now link to BP Trac not WP Plugin support forum
In 5160-grunt-build.diff
- Add default tasks to
grunt build
task- Cleans the
/build
folder withclean:all
task - Copies all files from
/src
to/build
withcopy:files
task - Creates RTL CSS files for all LTR CSS using
cssjanus:core
- Minimizes all CSS files with
cssmin:ltr
andcssmin:rtl
- Minimizes all JS files with
uglify:core
task - Creates
buddypress.pot
withmakepot
task`
- Cleans the
In 5160-remove-rtl-css.diff
- Remove all RTL CSS assets from
/src
- RTL CSS will now only be created when a build is produced.
- Grunt task
grunt cssjanus
destination directory should be /build - Changes
BP_CSS
toBP_LTR_CSS
andBP_RTL_CSS
- The change allows the dynamic
grunt watch
tasks to dynamically minimize and RTL only the required files that have been changed
- The change allows the dynamic
#45
in reply to:
↑ 15
@
11 years ago
The patches I added mimic the WordPress and bbPress repo setups as I outlined above in 5160#comment:15.
Some of the patches are dependant on other patches in particular renamed task, grunt build-dev
to grunt build
and the BP_CSS
to BP_LTR_CSS
and BP_RTL_CSS
change.
Questions? Fire away here or #IRC, more than happy to explain the above methods and/or madness ;)
#49
follow-up:
↓ 50
@
11 years ago
Thanks for your contributions, netweb. I've worked through them; some I've committed into trunk, but some need further discussion. I think generally we would like separate tickets for each proposed change to Grunt, as Trac makes it hard to have multiple conversations about multiple changes on the same ticket, and it's hard to keep track of them. ;)
Consistency with WordPress' Grunt process is a stronger argument than being consistent with bbPress', as bbPress' adoption of Grunt is still pretty new and will likely be iterated on as better ways of working are discovered. We shouldn't hold some idea that it's important for our build scripts to be 100% identical with our sister projects'; we can't let that stop us from adopting changes that work best for BuddyPress.
For the patches I'm about to discuss, unless you beat me to it, I'll open new tickets for each and move the patches and comments across, so we can discuss there.
5160-php56.diff / PHP 5.6
- Do you have any context for this change?
- If the tests are failing on 5.6, why? Aren't those bugs we need to fix? ;)
5160-makepot.diff
- AFAIK, we haven't turned off potbot yet.
- We shouldn't remove the
bp-languages
folder until that's done, because otherwise it might break things. bp_core_load_buddypress_textdomain()
also needs updating.- I don't think generating the POT on every main Grunt task is a good idea, because it's not very fast -- plus those changes aren't included in this patch?
- Switching the CWD from SOURCE to BUILD will cause trunk to not contain any POT file which will break GlotPress (the
build
folder is not being added to trunk). - From this patch, I think the idea to update the headers and move the POT to the root of
src
is great. Everything else, not so sure yet. :)
5160-remove-rtl-css.diff
- The core team had (unfortunately, in private -- sorry) previously decided to keep RTL CSS files in trunk, mainly to support contributions to the project from people who expect RTL.
- I think this change will be a
wontfix
, but a new ticket would be the place to have a discussion about this if you feel strongly.
5160-watch.diff
- This patch has a dependency on the
BP_LTR_CSS
change which I don't think is necessary for BuddyPress to adopt (that looks like it was originally taken from thebp-grunt-lite
experiment on Github, which has its own problems), and at any rate, that would have to be done before this patch could go in. - This is another great patch but it will have to be rewritten to remove dependencies on other patches, with careful consideration given to the effects of the proposed CWD change.
5160-grunt-build.diff
- Probably unlikely to be adopted initially until we have some real-world experience with how the existing workflow changes affect core development/contributions, before we iterate.
- I am particularly concerned with the extra time that the new
grunt
tasks would take to run, and why we'd want to generate a release build here:- For the record,
grunt
(akagrunt build
) is meant to be run periodically as you're working on a change. In essence, all of thegrunt:watch
tasks should also be run bygrunt build
. They're intended to be complementary, or a different way of doing the same thing. As such,grunt build
should be super super quick to run and not cause the person running the command to get annoyed by any delay. grunt build-commit
is meant to be run just before a committer commits a change. It's OK if this command takes longer to run; it doesn't need to be quick.grunt build-release
is meant to be run by a lead developer as we generate a alpha/beta/release version, or perhaps by anyone who wants to run the most cutting-edge BuddyPress without having all thesrc
junk, for whatever other reason. It can take as long as it needs to run, which is why this is the only default build command that runs our unit tests.
- For the record,
#50
in reply to:
↑ 49
@
11 years ago
Replying to DJPaul:
For the patches I'm about to discuss, unless you beat me to it, I'll open new tickets for each and move the patches and comments across, so we can discuss there.
5160-php56.diff / PHP 5.6
Migrated to #5620 - Travis CI: Including PHP 5.6 beta in test matrix
5160-makepot.diff
Migrated to #5621 - Remove buddypress.pot from develop repo
#51
@
11 years ago
5160-remove-rtl-css.diff
Migrated to #5622 - Remove RTL CSS in favor of only generating RTL CSS as part of the new build tools
#52
@
11 years ago
- Owner set to johnjamesjacoby
- Resolution set to fixed
- Status changed from new to closed
In 8392:
#53
@
11 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
Not actually fixed; mixed up the ticket numbers from patches on #5620.
#54
@
11 years ago
The build
directory is no longer created when running grunt
or grunt build
. Is this on purpose?
We shouldn't hold some idea that it's important for our build scripts to be 100% identical with our sister projects'; we can't let that stop us from adopting changes that work best for BuddyPress.
For the sanity of contributors to both projects, we should keep these processes as close as possible. Any changes that work for BuddyPress should ideally be implemented for bbPress. Something about a goose and a gander here.
#55
follow-up:
↓ 56
@
11 years ago
The build directory is no longer created when running grunt or grunt build
I don't know why you said "no longer". It never has been.
Is this on purpose?
Yes. grunt build-release
is what you want.
#56
in reply to:
↑ 55
;
follow-up:
↓ 59
@
11 years ago
Replying to johnjamesjacoby:
The
build
directory is no longer created when runninggrunt
orgrunt build
. Is this on purpose?
I found the same thing, albeit after I submitted the above patches ;)
I had 'assumed' it would use the /build
folder to 'load' similar to WordPress and bbPress implementations, thus 95% of what I submitted above was based on that assumption.
It wasn't until late this afternoon I realized this and the implementation here is for BuddyPress is to use the /src
folder by default to load BuddyPress as the 'normal' workflow when developing for BuddyPress.
I presume the only time BuddyPress would load the plugin via /build
would be to test for an upcoming release.
WordPress and bbPress need to load via /build
folder as 'we' have assets that are compiled by the build tools that are needed to load the full WordPress/bbPress package. Loading via the /src
folder is not possible because primarily of our SCSS assets needing to be compiled to CSS.
BuddyPress does not currently contain any 'assets' that must be compiled by the new build tools that would cause any issues to arise from running the plugin via /src
, WordPress and bbPress can not do this and maintain 100% functionality.
For the sanity of contributors to both projects, we should keep these processes as close as possible. Any changes that work for BuddyPress should ideally be implemented for bbPress. Something about a goose and a gander here.
Per the above, unless WordPress and bbPress commit code compiled by the build tools (SCSS, RTL, minified files etc) back to /src
, WordPress and bbPress cannot maintain 100% functionality loading via /src
.
#57
@
11 years ago
Below is an extract of my original reply I was going to submit before I realized what I wrote above in the previous comment.
I'm adding it as a personal opinion on how I see and use the build tools included in WordPress and bbPress.
(I have removed anything pertaining to the code changes I had previously submitted.)
Replying to DJPaul:
Consistency with WordPress' Grunt process is a stronger argument than being consistent with bbPress', as bbPress' adoption of Grunt is still pretty new and will likely be iterated on as better ways of working are discovered. We shouldn't hold some idea that it's important for our build scripts to be 100% identical with our sister projects'; we can't let that stop us from adopting changes that work best for BuddyPress.
Indeed they do not need to be identical and both should use the tools that best fit the needs of each project, that said, they need to be compatable, maybe complementary is a better term. Developing with bbPress and BuddyPress at the same is often needed by both projects. bbPress needs to ensure our BuddyPress integrations do not break and BuddyPress needs to ensure the bbPress forum implementation does not break.
Both bbPress and BuddyPress inherit initiatives from WordPress upstream such as Trac updates, inline docs, coding standards, contributing guidelines and this new build setup I don't see as any different, compatability, interoprability and consistency upstream is extremely important. bbPress' Grunt process is quite similar to WordPress' and I would go one step further and say 'in sync' for the most part, if I see improvements to be made I look to implement the changes in both bbPress and WordPress and that now would also extend to BuddyPress.
Dogfooding is the approach I have taken with both WordPress and bbPress 'develop' repo's. I see three components to the these Grunt powered repos.
- A folder which contains the core source files
- A folder which contains compiled source files
- A process to create a full release of the package for distribution.
Those descriptions above I purposefully left out the term 'build' as I see primarily the two tasks grunt build
and grunt build-release
as significantly serving two vastly different purposes. The later grunt build-release
runs all the grunt tasks needed to copy, minify, RTL, makepot, validate and unit test to facilitate the creation of a complete release to be deployed to the WordPress plugin SVN repo.
The former grunt build
, I don't see this as a tool to generate a 'release build', I see this as a tool to create a full working copy of the source with compiled assets that I use for primary development of bbPress (and WordPress). What this allows me to do is in the true spirit of dogfooding is to use the identical same package end users will use once released. I can set (or unset) define( 'SCRIPT_DEBUG', true );
in wp-config.php
as needed allowing for easy debugging of non minified CSS or JS assets, full RTL CSS is also available when testing with the RTL plugin tester for example.
- All of my WordPress 'trunk' installs use the
/build
folder as the documentroot/webroot, without it many asset types are not available such as MP6 themes - All of my bbPress development uses the
/build
folder as the plugin root, again, without the built project compiled MP6 themes are unavailable - The
/src
folder is used for all code changes with most changes deployed to the/build
folder live usinggrunt watch
or in bulk withgrunt build
#58
@
11 years ago
Agree with Stephen's sentiments above.
Can someone (Paul?) spell out what the benefits of having BuddyPress work differently than WordPress and bbPress are?
#61
in reply to:
↑ 59
@
11 years ago
Replying to DJPaul:
Replying to netweb:
WordPress and bbPress need to load via
/build
folder... Loading via the/src
folder is not possible because primarily of our SCSS assets needing to be compiled to CSS.
This is not true (for WordPress). I am running from develop.svn.wordpress.org/trunk/'s
/src/
just fine.
Checking out WordPress' develop.svn.wordpress.org/trunk/ repo will work without running the build tools and loading from the /src/
folder, this is by design for back compat for those WordPress /trunk users and the majority of WordPress' core functionality will work as expected but not all of it.
When loading only from /src
you do not have the MP6 themes as the SCSS has not been compiled into CSS nor do you have any WordPress RTL CSS.
On the left is WordPress running from /src
and on the right running from /build
#62
@
10 years ago
- Milestone changed from 2.1 to Future Release
I hate to re-open this can of worms, but I'm moving this to Future Release to clear 2.1 and interested parties can continue discuss the merits of changing our Grunt processes for future releases. I think what we have for 2.1 is pretty decent.
Maybe; it's worth looking into.