WordPress.org

Make WordPress Core

Opened 20 months ago

Closed 20 months ago

Last modified 20 months ago

#21633 closed task (blessed) (fixed)

Switch to min.js convention

Reported by: nbachiyski Owned by: nacin
Milestone: 3.5 Priority: normal
Severity: normal Version:
Component: General Keywords:
Focuses: Cc:

Description

Most of the projects I use these days use min.js for the filenames of their minified files. This does a couple of good things:

  • Sets some expectations in developers looking at the files. Numerous times I've forgotten for our .dev.js convention and have opened the minified files in a text editor.
  • Sets some expectations in some software tools. For example, ack automatically skips searching min.js files.

Possible problems: I don't see any.

Plugins should be using wp_enqueue_script() and changing paths shouldn't be a problem. Browser caches of developers shouldn't be a huge problem, either.

If everybody is fine with the change, I'd be happy to volunteer and steer the transition, including the patches.

Attachments (1)

21633.diff (1.3 KB) - added by georgestephanis 20 months ago.

Download all attachments as: .zip

Change History (17)

comment:1 follow-up: ocean9020 months ago

With this I would also give a vote for a new arg $dev in WP_SCRIPTS/WP_STYLES API which controls minified files. If $dev is defined in wp_register_script and SCRIPT_DEBUG is true load the minified file. This would reduce the checks in a plugin.

Version 0, edited 20 months ago by ocean90 (next)

comment:2 scribu20 months ago

wp_enqueue_script() already has too many parameters, IMO. We should switch to named arguments if we want to add $dev, or $min.

comment:3 follow-ups: nacin20 months ago

  • Milestone changed from Awaiting Review to 3.5
  • Type changed from feature request to task (blessed)

koopersmith and I were discussing this just yesterday. I strongly agree with this. It's also easy to do, and I don't think a patch would even be needed (beyond a few lines in script-loader), as most of the magic here is simply getting the svn copy commands right so history doesn't get all screwed up, like what happened in [10291].

Though implied, just going to state we should do this for CSS as well.

Plugins should be using wp_enqueue_script() and changing paths shouldn't be a problem.

Not only that, but no developer would really ever be referencing .dev.js directly. So if they were referencing .js directly, they'll simply go from using the minified to the unminified scripts.

With this I would also give a vote for a new arg $dev in WP_SCRIPTS/WP_STYLES API which controls minified files.

I like this, though I'm not sure how I feel about a new argument on wp_register/enqueue_script. There are probably three or four proposed new parameters for this function across Trac. I think it is time to convert $in_footer = false to an array of extra arguments, so 'footer' => true and the like. (As scribu just recommended as well.)

comment:4 in reply to: ↑ 3 koopersmith20 months ago

Replying to nacin:

koopersmith and I were discussing this just yesterday. I strongly agree with this.

Likewise. Nikolay hit the main points on the head — I think this is an easy win.

comment:6 in reply to: ↑ 3 ; follow-up: scribu20 months ago

Replying to nacin:

With this I would also give a vote for a new arg $dev in WP_SCRIPTS/WP_STYLES API which controls minified files.

I like this, though I'm not sure how I feel about a new argument on wp_register/enqueue_script. There are probably three or four proposed new parameters for this function across Trac. I think it is time to convert $in_footer = false to an array of extra arguments, so 'footer' => true and the like. (As scribu just recommended as well.)

Converting $in_footer to an array of extra arguments seems rather arbitrary to me, since only $handle and $src are required. I was thinking more along these lines:

wp_register_script( $handle, $src, array( ... ) );

comment:7 in reply to: ↑ 6 ; follow-up: azaozz20 months ago

Replying to scribu:

That's not going to work well. Currently:

wp_register_script( $handle, $src, $deps = array(), ... );

so we would be fishing in the $deps array for other args.

comment:8 in reply to: ↑ 1 azaozz20 months ago

Replying to ocean90:

With this I would also give a vote for a new arg $dev in WP_SCRIPTS/WP_STYLES API which controls minified files. If $dev is defined in wp_register_script and SCRIPT_DEBUG is true load the minified file.

You mean the non-minified file.

The same could probably be accomplished by looking for .min.js or .min.css in the registered script or stylesheet and expecting there are non-minified versions without the suffix.

comment:9 kurtpayne20 months ago

  • Cc kpayne@… added

comment:10 in reply to: ↑ 7 scribu20 months ago

Replying to azaozz:

so we would be fishing in the $deps array for other args.

Yeah, you're right. So maybe after it:

wp_register_script( $handle, $src, $deps = array(), $args = array() );

The same could probably be accomplished by looking for .min.js or .min.css in the registered script or stylesheet and expecting there are non-minified versions without the suffix.

Yes, but only if we ship non-minified versions of all the third-party libs, like jQuery, which we don't.

comment:11 nbachiyski20 months ago

Notes on the implementation:

The goal is to retain the history for all files. Here is what I came up with:

svn cp xxx.js xxx.min.js
svn rm xxx.js
svn mv xxx.dev.js xxx.js

svn is smart enough to notice we are replacing files. Here is the svn st:

A  +    xxx.min.js
R  +    xxx.js
D       xxx.dev.js

In my testing this retains the history for all of the files. I would love if some of your tests on their setups to see if that's true.

Here's a dirty script to do all the needed changes:

find . -name '*.dev.js' -print0 | xargs -0 perl -e 'foreach(@ARGV){ $dev=$_; $min=$js=$dev; $js=~s/\.dev\././; $min=~s/\.dev\./.min./; system("svn cp $js $min"); system("svn rm $js"); system("svn mv $dev $js"); }'

comment:12 nacin20 months ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In [21592]:

Switch to .min for compressed JS and CSS files.

  • This moves our "development" versions from .dev.js to .js (same for css).
  • The compressed version then moves from .js to .min.js (same for css).

By switching to the standard .min convention, it sets expectations for developers,
and works nicely with existing tools such as ack.

fixes #21633.

comment:13 nacin20 months ago

Thanks for the legwork, Nikolay. Rather than svn cp; svn rm; svn mv, it only required svn mv; svn mv. This makes SVN consider the file "replaced." If you're interested, http://www.red-bean.com/pipermail/svnbook-dev/2006-January/001527.html.

georgestephanis20 months ago

comment:14 georgestephanis20 months ago

Minor patch to remedy two internal references to .dev.js files.

comment:15 nacin20 months ago

In [21659]:

Update some code comments from .dev.js to .js. props georgestephanis. see #21633.

comment:16 nacin20 months ago

In [21660]:

Update old files array:

  • Remove old names for jQuery UI Effects files. see #21736.
  • Remove un-minified version of Jcrop and jQuery Color. see #21692, #20728.
  • Remove .dev.css and .dev.js. see #21633.
Note: See TracTickets for help on using tickets.