Make WordPress Core

Opened 8 years ago

Closed 7 years ago

#38806 closed defect (bug) (duplicate)

Better wp namespace in customize-base.js

Reported by: afercia's profile afercia Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Customize Keywords: needs-patch
Focuses: javascript Cc:

Description

Currently, customize-base.js is using this pattern to pass the wp object as argument:

window.wp = window.wp || {};

(function( exports, $ ){

...

})( wp, jQuery );

while the wp passed to the IIFE should be window.wp.

Change History (6)

#1 @westonruter
8 years ago

@afercia Is there a practical reason for this other than a style thing? Is an editor complaining because wp is not defined in the file? It is defined as a global in .jshintrc so that's why it's not been a problem for jshint so far.

#2 @afercia
8 years ago

@westonruter yep I know grunt jshint doesn't catch it because it's set as global in .jshintrc. I was wondering why there's a so great variety of different conventions across all the JS files, this can be confusing for new contributors. So here, window.wp = window.wp || {}; is used just to make sure the object is created, then wp is passed regardless if it's defined or not.

In many other cases window.wp is explicitly passed. In other cases nothing is expliicitly passed, see for example password-strength-meter.js. In other cases again window.wp = window.wp || {}; is used inside the IIFE, which is OK but sometimes it's after other references to wp.

I was going to open a ticket for each case, but honestly they're so many that I gave up :) Thinking that establishing a convention would have some value though.

#3 @westonruter
8 years ago

@afercia agreed. This would also likely be addressed by a big effort to split up the monolithic JS files in #30277.

#4 @peterwilsoncc
8 years ago

I'd like to see a consistent pattern across all files too, if nothing else to avoid unnecessary scope traversal.

This is my pattern of choice as it protects against accidental defining of undefined.

( function( window, undefined ) {
        window.wp = window.wp || {};
        var document = window.document;
        var $ = window.jQuery;
        var wp = window.wp;
})( window );

#5 @afercia
8 years ago

Yep, there are also a few cases of passing undefined, but just in a few files, see for example:

wp-admin/js/common.js
wp-admin/js/editor-expand.js
wp-includes/js/heartbeat.js
wp-includes/js/wp-api.js

#6 @westonruter
7 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed

We'll address this as part of #30277, and breaking up the customizer JS into modules.

Note: See TracTickets for help on using tickets.