WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 years 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 3 years ago.

Download all attachments as: .zip

Change History (17)

comment:1 follow-up: @ocean903 years 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 non-minified file. This would reduce the checks in a plugin.

Last edited 3 years ago by ocean90 (previous) (diff)

comment:2 @scribu3 years 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: @nacin3 years 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 @koopersmith3 years 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: @scribu3 years 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: @azaozz3 years 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 @azaozz3 years 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 @kurtpayne3 years ago

  • Cc kpayne@… added

comment:10 in reply to: ↑ 7 @scribu3 years 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 @nbachiyski3 years 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 @nacin3 years 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 @nacin3 years 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.

@georgestephanis3 years ago

comment:14 @georgestephanis3 years ago

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

comment:15 @nacin3 years ago

In [21659]:

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

comment:16 @nacin3 years 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.