Skip to content

Improve Customizer JSDoc - #10743

Closed
westonruter wants to merge 48 commits into
WordPress:trunkfrom
westonruter:trac-40831
Closed

westonruter wants to merge 48 commits into
WordPress:trunkfrom
westonruter:trac-40831

Conversation

@westonruter

@westonruter westonruter commented Jan 15, 2026

Copy link
Copy Markdown
Member

✅ Committed in:


Trac ticket: https://core.trac.wordpress.org/ticket/40831


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

westonruter and others added 2 commits January 15, 2026 13:21
Co-authored-by: shailu25 <shailu25@git.wordpress.org>
Co-authored-by: vishalkakadiya <vishalkakadiya@git.wordpress.org>
Per Gemini:

I've completed the JSDoc improvements in the `src/js/_enqueues/wp/customize/` directory.

Summary of changes:
- Corrected missing braces around types in `@param` and `@return` tags across several files.
- Replaced non-standard return types like `{wp.customize.controlConstructor.menus[]}` with more accurate instance types like `{wp.customize.Control}` or `{wp.customize.Control[]}`.
- Fixed placeholder JSDoc like `[type]` and `[description]` in `base.js`.
- Updated descriptions to use "jQuery object" instead of "jQuery collection" for consistency.
- Improved formatting for nested parameters in `Messenger.initialize`.
- Corrected a parameter name mismatch in `api.Class.extend`.

All changes have been verified with `svn diff`.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@github-actions

github-actions Bot commented Jan 15, 2026

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props westonruter, mukesh27, afercia.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@westonruter westonruter changed the title Improve Customizer jsdoc Improve Customizer JSDoc Jan 15, 2026
@github-actions

Copy link
Copy Markdown

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • The Plugin and Theme Directories cannot be accessed within Playground.
  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

Gemini: I have completed the requested JSDoc improvements for the Customizer JavaScript files based on the requirements of ticket #40831. All local changes are confined to JSDoc blocks and have been verified. I am now finished with the task.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request improves JSDoc documentation across multiple WordPress Customizer JavaScript files. The changes add missing documentation blocks, clarify parameter types, add return type annotations, and update imprecise type references to more accurate generic types.

Changes:

  • Added comprehensive JSDoc comments for previously undocumented methods in views, models, and loader files
  • Updated return type annotations from specific control constructor types to generic wp.customize.Control types for better accuracy
  • Added missing @return {void} tags for event handler methods and @since tags where appropriate

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/js/_enqueues/wp/customize/widgets.js Updated return types for control-related methods from specific constructor types to generic Control types
src/js/_enqueues/wp/customize/views.js Added comprehensive JSDoc for HeaderTool view methods including initialize, render, and helper functions
src/js/_enqueues/wp/customize/preview.js Added parameter documentation for debounce function and return type tags for event handlers
src/js/_enqueues/wp/customize/preview-nav-menus.js Added @SInCE tags, parameter documentation, and return type annotations for nav menu preview functions
src/js/_enqueues/wp/customize/nav-menus.js Updated return types from specific control constructor types to generic Control types and improved parameter documentation
src/js/_enqueues/wp/customize/models.js Added comprehensive JSDoc for HeaderTool model methods including initialize, comparator, and utility functions
src/js/_enqueues/wp/customize/loader.js Added JSDoc for event handler methods and improved parameter documentation for state management functions
src/js/_enqueues/wp/customize/base.js Improved parameter and return type documentation for core utility functions and classes

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/js/_enqueues/wp/customize/preview-nav-menus.js
Co-authored-by: Mukesh Panchal <mukeshpanchal27@users.noreply.github.com>
Comment thread src/js/_enqueues/wp/customize/preview-nav-menus.js
westonruter and others added 9 commits August 28, 2026 14:30
Fold in the JSDoc corrections made to `src/js/_enqueues/wp/customize/` in
WordPress#13251 so that those files
can be dropped from that pull request.

Where that pull request removed a `@param` tag because the documented variadic
had no corresponding named parameter for `jsdoc/check-param-names` to match,
adopt rest syntax instead of dropping the documentation. This preserves — and
in several cases restores — the description of what the extra arguments mean:

* `wp.customize.Value#bind()`, `#unbind()`, `#link()`, `#unlink()`, `#sync()`
  and `#unsync()`. The latter four previously carried only a trailing
  `// values*` comment, now replaced by real `@param` tags.
* `wp.customize.Values#instance()`, `#create()` and `#when()`.
* `wp.customize.Events#trigger()`, `#bind()` and `#unbind()`, which gain
  docblocks they never had.

Convert the remaining uses of `arguments` in these files to rest parameters, or
to a direct `call()` where the receiving method declares a fixed signature. Two
of these are worth noting:

* `wp.customize.Widgets.WidgetControl` forwards to a `widget-synced` handler
  that takes a third `newForm` argument the listener does not declare, so it
  keeps forwarding via rest rather than collapsing to a fixed `call()`.
* `wp.customize.Class` keeps `arguments`, since it is passed on to
  `initialize()` and must reflect the number of arguments actually supplied.

Replace `wp.customize.controlConstructor.*` return types, which name a
constructor where an instance is meant, with the documented control classes:
`wp.customize.Menus.MenuControl`, `wp.customize.Menus.MenuItemControl`,
`wp.customize.Widgets.SidebarControl` and `wp.customize.Widgets.WidgetControl`.

`Array.prototype.slice` is no longer referenced in customize-base.js and is
removed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`customize-base.js` wraps its contents in an IIFE whose first parameter is
named `exports`, because that is precisely what it is used for:

    (function( exports, $ ){
        …
        exports.customize = api;
    })( wp, jQuery );

That header was copied to `customize-controls.js`, `customize-preview.js` and
`customize-loader.js`, but in those three files the parameter is never
referenced. Each one reaches for the global `wp` instead, so the argument
being passed in is silently discarded.

Rename the parameter to `wp` in those three files so that the argument is
actually consumed and the global lookups resolve to a local binding. This
matches `customize-widgets.js`, which already declares `(function( wp, $ ){`.

`customize-base.js` is left alone, as `exports` is both used there and
descriptive of its role.

Co-Authored-By: Andrea Fercia <afercia@git.wordpress.org>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`wp.customize.Class` accepts arguments in two forms. Normally they are passed
straight through to the class's `initialize` method, which is how nearly every
instance in the Customizer is constructed. As a special case, when the first
argument is `wp.customize.Class.applicator`, the second argument is the array
of arguments for `initialize` and the third extends the instance.

Collecting the direct form requires the number of arguments actually supplied,
which is why `arguments` was used here. Declaring the three named parameters
and collecting only the remainder with a rest parameter would not preserve
that: rebuilding the list as `[ applicator, argsArray, options, ...rest ]`
always yields at least three entries, so `new wp.customize.Value( true )` would
call `initialize()` with three arguments rather than one. Collecting every
argument with a single rest parameter and indexing into it instead is exactly
equivalent, since the resulting array has the original length.

Document both forms while here. The previous docblock described only the
applicator form, which has a single call site, and not the direct form used
everywhere else.

Also rename the rest parameter of the `instance` wrapper, which shadowed the
outer `args`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The docblocks added for `link()` and `unlink()` described the relationship
backwards, saying that the value's changes are propagated to the supplied
values. It is the other way around: `link()` binds this value's setter as a
callback on each supplied value, so this value follows them.

The call sites read that way too. `Messenger` derives `origin` from `url`, an
input element follows its setting, and the selected changeset status follows
the changeset status so that updates made on the server are reflected in the
selection.

Note the one-directional nature of this, since `sync()` is the method that
links in both directions.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The existing tests exercised these classes only through their simplest calls,
passing a single callback or a single value, so nothing confirmed the behavior
of the methods that accept any number of arguments. Add tests for:

* `Class.applicator`, resolving a longstanding `@todo`. One test covers the
  arguments being taken from the supplied array, and another covers the
  instance being extended before `initialize()` runs.
* The number of arguments `Class` passes to `initialize()`, which has to match
  the number it was given.
* An instance being callable as a function when the class defines an
  `instance()` method, resolving the other `@todo`.
* `Value#bind()` and `Value#unbind()` with more than one callback.
* `Value#link()` and `Value#unlink()`, including that following a value is
  one-directional, and `Value#sync()` and `Value#unsync()`.
* `Values#create()` passing its extra arguments through to `initialize()`.
* `Values#when()` waiting for a value that does not exist yet.

The suite wraps every test with sinon's fake timers, so the promise returned by
`when()` does not resolve until the clock is advanced. Advance it rather than
waiting, which keeps the test synchronous.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`mixed` is a PHP type. JSDoc spells the any type as `*`, and TypeScript, which
checks a growing number of the files in `src/js` by way of `tsconfig.json`,
reports `Cannot find name 'mixed'` for it.

Ten occurrences across customize-base.js, customize-controls.js and
customize-views.js are updated, and the surrounding parameter descriptions are
realigned.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Several types were written as bare class names, or in terms of the `api` alias
that the Customizer files use internally for `wp.customize`. Neither form
resolves, since the documented names are the public ones:

* `{Value}` becomes `{wp.customize.Value}`.
* `{Placement}` and `{Partial}` become
  `{wp.customize.selectiveRefresh.Placement}` and
  `{wp.customize.selectiveRefresh.Partial}`.
* `{api.Notification}` becomes `{wp.customize.Notification}`, and
  `{api.selectiveRefresh.Placement}` becomes its `wp.customize` equivalent.
* `@see {api.Values.when}` becomes `@see {@link wp.customize.Values#when}`,
  matching how the other cross references in these files are written.

Two `@lends` annotations in customize-selective-refresh.js named the wrong
symbol, so the members they introduce were attached to something that does not
exist. `Partial` used `wp.customize.SelectiveRefresh`, which is capitalized
differently to the `wp.customize.selectiveRefresh` namespace it belongs to, and
`Placement` lent its members to the namespace itself rather than to
`Placement`.

Also correct two malformed types. `{event}` is not a type; the parameter is a
jQuery event, as it is everywhere else in customize-controls.js. And a default
value belongs on the parameter name rather than inside the braces, so
`{boolean=true} [options.triggerRendered]` becomes `{boolean}
[options.triggerRendered=true]`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The `$` argument of these functions is jQuery itself, not a collection of
elements, so `{jQuery}` describes the wrong thing. The type of the `jQuery`
global is `JQueryStatic`, which is the name `@types/jquery` exports and the one
`typings/wp-globals/index.d.ts` already refers to.

The `wp` and `_` arguments were given `{wp}` and `{_}`, which name the globals
being passed rather than any type. These become `{Object}`, matching how
customize-preview-widgets.js already describes the same two arguments.

For the same reason `{window}` becomes `{Window}` in the Messenger docblock,
naming the interface rather than the global.

While here, move the arguments of the customize-views.js function out of the
file's `@output` docblock and into one of its own, attached to the function.
The other files here already keep the two separate.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment thread src/js/_enqueues/wp/customize/models.js Outdated
},

/**
* Maybe add random choice.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love the determinism of this!!! 🤪

westonruter and others added 2 commits August 28, 2026 17:39
Found by running the Customizer files through a stricter set of the rules that
`eslint-plugin-jsdoc` offers than `.eslintrc-jsdoc.js` currently enables,
notably `no-undefined-types`, `valid-types`, `check-access`, `check-alignment`
and `no-bad-blocks`.

* The `@callback` definitions for the deferred control, section, panel and
  notification callbacks are declared under `wp.customize`, but were referred
  to by their bare names, which do not resolve.

* `params.message=null` gave a default for a parameter that was not marked
  optional, which is a namepath syntax error. The parameter is optional, since
  `initialize()` defaults it to null.

* `wp.customize.addLinkPreviewing()` carried both `@access protected` and
  `@access private`. The surrounding functions in customize-preview.js use
  `@access protected`.

* The properties of `wp.customize.selectiveRefresh.Placement` were given their
  types with `@param`, which documents a parameter rather than a member. These
  become `@member`, as in `wp.customize.Notification`.

* The file header of customize-preview.js opened with `/*` rather than `/**`,
  so its `@output` was not a documentation comment at all. It is now, and the
  arguments of the function below it move into a docblock of their own, as in
  the other files here.

* Four docblocks in customize-widgets.js were indented with a stray space.

Also describe the parameters that were left undescribed in the docblocks this
branch already touches, in customize-controls.js,
customize-selective-refresh.js and customize-widgets.js.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The JavaScript Documentation Standards give no separator between a parameter
name and its description, and none of the examples there use one. Sixty-eight
`@param` tags across six of these files did, so they are brought into line and
the description column is realigned.

Only the separator is removed. Hyphens that belong to a description are left
alone.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@westonruter
westonruter requested review from afercia and a lite review from Copilot August 29, 2026 00:55

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.

Comment thread src/js/_enqueues/wp/customize/controls.js Outdated
Comment thread src/js/_enqueues/wp/customize/widgets.js Outdated
Comment thread src/js/_enqueues/wp/customize/controls.js Outdated
Comment thread src/js/_enqueues/wp/customize/selective-refresh.js Outdated
Comment thread src/js/_enqueues/wp/customize/selective-refresh.js Outdated
@westonruter
westonruter requested a review from afercia September 1, 2026 16:47
westonruter and others added 5 commits September 1, 2026 10:09
Thirty-three annotations described a jQuery collection as `{jQuery}`. That
spelling does resolve, which is why nothing complained, but it resolves to
the wrong thing: `jQuery` is declared as a variable of type `JQueryStatic`,
so a reader or a type checker is told these values are the `$` function
itself. The collection type is `JQuery`.

Checked against the repository's own tsconfig.json:

    /** @type {jQuery} */ → Property does not exist on type 'JQueryStatic'
    /** @type {JQuery} */ → Property does not exist on type 'JQuery<HTMLElement>'

Every one of the thirty-three holds a collection, and several say so in
their own description, such as `@return {jQuery} The jQuery collection.`
in `api.ensure()`. This is the same correction already made for
`jQuery.Promise`, and it leaves the file consistent with the `JQueryStatic`,
`JQuery.Event` and `JQuery.Promise` spellings already in use.

Only the type positions are touched. Prose that talks about a jQuery object
or collection still says jQuery, and the two names are the same length, so
no alignment moves.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three overrides of `selectiveRefresh.Partial.prototype.refresh()` documented
their return as `{Promise}`. Each one returns `$.Deferred().promise()`, a
jQuery promise, and the base method they override already documents
`{JQuery.Promise<*>}`. `Promise` names the native constructor, which has a
different API: a jQuery promise carries `done`, `fail` and `always`, and its
`then` predates the Promises/A+ signature.

`WidgetPartial.refresh()` additionally offered `{Promise|void}`. It never
returns void. Both branches return: one the rejected deferred it just built,
the other the result of the base implementation, which itself always returns
its `refreshPromise`.

The descriptions are reworded to say what each promise settles on, since
"A promise postponing the refresh" described the return by its effect on the
caller rather than by what it resolves to.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`handleFieldActiveToggle()` was annotated `@this {jQuery}`, but it is passed
to `fieldActiveToggles.each()` and bound with `.on( 'click', … )`, and jQuery
sets `this` to the raw DOM element in both. The function body settles it: it
calls `$( this ).val()` and `$( this ).prop( 'checked' )`, and wrapping would
be pointless if `this` were already a collection.

The elements are the `.hide-column-tog` checkboxes that WP_Screen renders as
`<input type="checkbox">`, so the type is `HTMLInputElement`, which is also
what makes `val()` and the `checked` property meaningful here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`WidgetsPanel` lent its prototype to `wp.customize.Widgets.WigetsPanel`,
missing the `d`. The class is spelled `WidgetsPanel` in its own `@class` tag
two lines above, in the assignment, and in the panelConstructor registration,
so every method in that literal was being documented onto a namespace that
exists nowhere else. This is the same slip as the `ThemsPanel` one corrected
earlier in this branch.

`SidebarPartial` tagged its constructor `@class`. The docblock sits on
`initialize`, so `@class` declares `initialize` itself to be the class;
`@constructs` is what marks a function as the constructor of the class its
prototype is being lent to. Of the nineteen constructor docblocks in these
files, this was the only one not using `@constructs` — its sibling
`WidgetPartial`, seventy lines earlier in the same file, already did.

`api.Value` documented no base class despite extending `api.Class`. Its
neighbours in the same file all record theirs: `Values`, `Messenger` and
`Notification` augment `wp.customize.Class`, and `Element` augments
`wp.customize.Value`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`Partial` and `Placement` sit side by side in one file and declared
themselves differently: `Partial` used a bare `@class` and `Placement` wrote
`@class Placement`. That is the only unqualified name given to `@class` in
these files; the other forty-six are either bare alongside `@memberOf`, or
fully qualified without one.

The difference turns out to be load-bearing rather than stylistic. A bare
`@class` makes JSDoc infer the name from the code, and the two chain their
assignments in opposite directions:

    Partial = self.Partial = api.Class.extend( … )
    self.Placement = Placement = api.Class.extend( … )

So the inference reads `Partial` for the first, which is right, and
`self.Placement` for the second, which yields the longname
`wp.customize.selectiveRefresh.self.Placement`. Simply dropping the name to
match its neighbour would have broken the class.

Both now carry an explicit `@alias`, the form already used for the partial
subclasses in preview-nav-menus.js and preview-widgets.js, so neither
depends on the order its assignment happens to be written in. Running JSDoc
over the file before and after yields the same eighty-eight documented
symbols.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@afercia afercia left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only minor thing I see is that several verbs that start a description should be third person. For example:

  • Initialize
  • Trigger
  • Bind
  • Unbind
  • Get
  • Set
  • Update
  • Stop
  • Link
  • Create
  • Handle
  • Push
  • Show
  • Hide
  • Add

For the rest, I totally defer to you.

Comment thread src/js/_enqueues/wp/customize/widgets.js Outdated
Comment thread src/js/_enqueues/wp/customize/widgets.js Outdated
Comment thread src/js/_enqueues/wp/customize/widgets.js Outdated
@afercia

afercia commented Sep 2, 2026

Copy link
Copy Markdown
Member

In other JS files we now have a few occurrences of:

  • {jQuery.Promise} lowercase j
  • {jQuery.promise} lowercase j and p
  • {jQuery.Event} lowercase j

Should we fix them in another PR?

@westonruter

Copy link
Copy Markdown
Member Author

In other JS files we now have a few occurrences of:

Yes, let's keep the changes here limited to the Customizer JS files.

westonruter and others added 4 commits September 2, 2026 16:26
Sixteen descriptions were fragments rather than sentences: some began in
lower case and ran on without a period, some restated the parameter name and
said nothing, and one described what a parameter accepts by writing out the
two literals, `1|-1`, with no indication of what moving by one means.

Reviewing the whole set rather than the three that were pointed out, the two
shapes turn out to be exhaustive across these files. Every remaining tag
description now starts with a capital and ends with a period, and none is
left empty where the block already documents the value's siblings.

The three offsets are described in terms of what they move, since the caller
of `_changeDepth()` cannot tell from `1|-1` that the sign selects a direction
rather than an amount, and the function throws for anything else.

Only the one prose summary that started in lower case is touched, on the
`selectSidebarItem()` helper, where the verb is also put in the third person
to match the surrounding blocks. The remaining summaries that lack a closing
period are left alone; they are a much larger set and a separate question.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`getInitialHeaderImage()` documented `@return {Object} Options`, which is the
return of `calculateImageSelectOptions()`, the method that sits either side
of it in this file and feeds options to the imgAreaSelect plugin. This one
returns a model: every path ends in `new api.HeaderTool.ImageModel( … )`,
either empty when no header image is set or populated from the matching
upload. Its own summary already said as much, and contradicted the tag
directly below it.

Found while normalizing the wording of the three `@return {Object} Options`
tags, where this one turned out not to belong to that group at all.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Forty-nine `@param` tags carried a type and a name and stopped there. A
reader learning what to pass had only the parameter's own name to go on, and
for the ones named `a`, `b`, `args` or `params` that is nothing at all.

Where a method aliases or overrides one that is already documented, the
existing wording is reused rather than reinvented, so the two now read the
same: the `Panel.onChangeExpanded()` and `Control.onChangeActive()` tags come
from their counterparts on Section and Container, and the `expand()` and
`collapse()` aliases in nav-menus.js and widgets.js from the Container
methods they point at.

The rest are described from the code. `_children()` is explained in terms of
its two call sites, `_children( 'section', 'control' )` and
`_children( 'panel', 'section' )`, which show that the first argument names
the Value on each child holding its parent's ID while the second names the
collection on wp.customize to walk. `setImageFromURL()` and the `widgetId`
helpers are described by what their bodies do with the value.

No alignment moves: every block already had columns wide enough, so the
change is forty-nine lines replaced one for one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`Container.onChangeActive()` said its argument was the active state to
"transiution" to.

`Control.onChangeActive()` did not document `args.unchanged`, though the
first thing its body does is branch on it and return early. The override now
documents the same five values as the Container method it mirrors, in the
same order and wording, so the two can be read against each other.

Four arguments were marked required that the code treats as optional. Three
of them, `attachmentId`, `width` and `height` on `setImageFromURL()`, are
each copied into the data object only inside an `if`, and the `options` of
`Value`, `Messenger` and `Previewer` reach `$.extend()` through `options ||
{}`, so omitting any of them is ordinary usage rather than a mistake.

Deliberately left required: `Control.initialize()` reads `options.params ||
options || {}`, which dereferences the argument before the fallback can
apply, so calling it with nothing throws rather than defaulting. For the same
reason `Placement.initialize()` stays required, since the `args || {}` there
only defers the failure to the `args.partial` check that throws two lines
later.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

It touches core Customizer runtime modules across many files (not just comments) and warrants human verification against build/tooling expectations and runtime compatibility.

Review details
  • Files reviewed: 12/12 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@afercia afercia left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, the documentation has greatly improved. LGTM.
For the code changes, I defer to @westonruter

wporg-sync pushed a commit that referenced this pull request Sep 3, 2026
The variadic signatures could not be documented without names for their arguments, so the `arguments` object is replaced with modern rest parameters across `wp.customize.Class`, `Events`, `Value`, `Values`, `Element` and `Messenger`. The `Array.prototype.slice` alias they relied on is no longer needed and is removed. Rest parameters are supported by >96% of users globally and all browser versions supported by WordPress.  

The argument these two files receive was named `exports`, while the docblock beside the line using it said `window.wp.customize`. It is now `wp`, and the arguments of both wrapping functions are documented.

Many of the types named nothing that resolves. Seven tags were typed `mixed`, which is not a JSDoc type; one still carried the `[type]` placeholder an IDE had left behind; and one each read `...`, an unqualified `Value`, and `string|jQuery collection`, which is two words rather than a type expression. Where the jQuery namespace appeared it was spelled in lower case, so it named the `$` function rather than a collection or an event. The value accepted by `Value#set()` was typed as an object, though a `Value` holds anything. In loader.js the one documented parameter had no braces at all.

The remaining docblocks are new, since most of these methods carried none. Among what they now record: `Class` returns a function rather than an object whenever the prototype defines an instance method, which is how `wp.customize()` and `wp.customize.control()` come to be callable, and `Value#link()` binds the receiver to follow the values it is given rather than the reverse.

QUnit tests cover the applicator form of the constructor, the forwarding of arguments through the event methods, and `unsync()` in both directions, resolving two long-standing todos in that file.

Developed as subset of #10743.
Follow-up to r48110, r48650.

Props westonruter, afercia, grapplerulrich, vishalkakadiya, shailu25.
See #39671, #40831, #64662, #66033.


git-svn-id: https://develop.svn.wordpress.org/trunk@63454 602fd350-edb4-49c9-b593-d223f7449a82
markjaquith pushed a commit to markjaquith/WordPress that referenced this pull request Sep 3, 2026
The variadic signatures could not be documented without names for their arguments, so the `arguments` object is replaced with modern rest parameters across `wp.customize.Class`, `Events`, `Value`, `Values`, `Element` and `Messenger`. The `Array.prototype.slice` alias they relied on is no longer needed and is removed. Rest parameters are supported by >96% of users globally and all browser versions supported by WordPress.  

The argument these two files receive was named `exports`, while the docblock beside the line using it said `window.wp.customize`. It is now `wp`, and the arguments of both wrapping functions are documented.

Many of the types named nothing that resolves. Seven tags were typed `mixed`, which is not a JSDoc type; one still carried the `[type]` placeholder an IDE had left behind; and one each read `...`, an unqualified `Value`, and `string|jQuery collection`, which is two words rather than a type expression. Where the jQuery namespace appeared it was spelled in lower case, so it named the `$` function rather than a collection or an event. The value accepted by `Value#set()` was typed as an object, though a `Value` holds anything. In loader.js the one documented parameter had no braces at all.

The remaining docblocks are new, since most of these methods carried none. Among what they now record: `Class` returns a function rather than an object whenever the prototype defines an instance method, which is how `wp.customize()` and `wp.customize.control()` come to be callable, and `Value#link()` binds the receiver to follow the values it is given rather than the reverse.

QUnit tests cover the applicator form of the constructor, the forwarding of arguments through the event methods, and `unsync()` in both directions, resolving two long-standing todos in that file.

Developed as subset of WordPress/wordpress-develop#10743.
Follow-up to r48110, r48650.

Props westonruter, afercia, grapplerulrich, vishalkakadiya, shailu25.
See #39671, #40831, #64662, #66033.

Built from https://develop.svn.wordpress.org/trunk@63454


git-svn-id: http://core.svn.wordpress.org/trunk@62634 1a063a9b-81f0-0310-95a4-ce76da25c4cd
wporg-sync pushed a commit that referenced this pull request Sep 3, 2026
Many of the types named nothing that resolves. The four deferred getters referred to their callback typedefs without a namespace, the promise they return was spelled `jQuery.promise`, which is neither the capitalization the namespace is declared under nor the name of a type, and jQuery events were typed as plain objects.

Two blocks documented the wrong thing outright. `ThemesPanel` lent its prototype to a name misspelled without the second `e`, so its methods were attached to a namespace that exists nowhere else, and `getInitialHeaderImage()` carried the return of the method beside it, describing a set of cropper options where its own summary already said it returns a model.

Parameters the code treats as optional are marked as such.

Developed as subset of #10743.
Follow-up to r41799, r48650, r63454.

Props westonruter, afercia.
See #39671, #39930, #40831, #64662, #66033.


git-svn-id: https://develop.svn.wordpress.org/trunk@63455 602fd350-edb4-49c9-b593-d223f7449a82
markjaquith pushed a commit to markjaquith/WordPress that referenced this pull request Sep 3, 2026
Many of the types named nothing that resolves. The four deferred getters referred to their callback typedefs without a namespace, the promise they return was spelled `jQuery.promise`, which is neither the capitalization the namespace is declared under nor the name of a type, and jQuery events were typed as plain objects.

Two blocks documented the wrong thing outright. `ThemesPanel` lent its prototype to a name misspelled without the second `e`, so its methods were attached to a namespace that exists nowhere else, and `getInitialHeaderImage()` carried the return of the method beside it, describing a set of cropper options where its own summary already said it returns a model.

Parameters the code treats as optional are marked as such.

Developed as subset of WordPress/wordpress-develop#10743.
Follow-up to r41799, r48650, r63454.

Props westonruter, afercia.
See #39671, #39930, #40831, #64662, #66033.

Built from https://develop.svn.wordpress.org/trunk@63455


git-svn-id: http://core.svn.wordpress.org/trunk@62635 1a063a9b-81f0-0310-95a4-ce76da25c4cd
wporg-sync pushed a commit that referenced this pull request Sep 3, 2026
Three blocks described something the code does not do.

1. `WidgetsPanel` lent its prototype to `WigetsPanel`, so its methods were attached to a name that appears nowhere else in core.
2. `WidgetControl.onChangeExpanded()` said its arguments were merged over `defaultActiveArguments`, while the handler that calls it merges over `defaultExpandedArguments`; the two carry separate defaults and separate queues, so the wrong one points a reader at the wrong callback.
3. The field toggle handler in the menu section annotated `this` as a jQuery object, when it is used with `each()` and a click binding, both of which set `this` to the raw element its body then wraps.

The `offset` parameters described what they accept by writing out the two literals it may hold, which does not convey that the sign selects a direction rather than an amount.

Developed as subset of #10743.
Follow-up to r48650, r63454, r63455.

Props westonruter, afercia.
See #39671, #40831, #64662, #66033.


git-svn-id: https://develop.svn.wordpress.org/trunk@63456 602fd350-edb4-49c9-b593-d223f7449a82
markjaquith pushed a commit to markjaquith/WordPress that referenced this pull request Sep 3, 2026
Three blocks described something the code does not do.

1. `WidgetsPanel` lent its prototype to `WigetsPanel`, so its methods were attached to a name that appears nowhere else in core.
2. `WidgetControl.onChangeExpanded()` said its arguments were merged over `defaultActiveArguments`, while the handler that calls it merges over `defaultExpandedArguments`; the two carry separate defaults and separate queues, so the wrong one points a reader at the wrong callback.
3. The field toggle handler in the menu section annotated `this` as a jQuery object, when it is used with `each()` and a click binding, both of which set `this` to the raw element its body then wraps.

The `offset` parameters described what they accept by writing out the two literals it may hold, which does not convey that the sign selects a direction rather than an amount.

Developed as subset of WordPress/wordpress-develop#10743.
Follow-up to r48650, r63454, r63455.

Props westonruter, afercia.
See #39671, #40831, #64662, #66033.

Built from https://develop.svn.wordpress.org/trunk@63456


git-svn-id: http://core.svn.wordpress.org/trunk@62636 1a063a9b-81f0-0310-95a4-ce76da25c4cd
wporg-sync pushed a commit that referenced this pull request Sep 3, 2026
Missing tags and descriptions are added, and the types are corrected, whether they named nothing resolvable (`Promise` for `JQuery.Promise<*>`), said too little (`Array` for `string[]`), or were narrower than what the code passes.

Both classes in selective-refresh.js lent their prototypes to the wrong name: `Partial` named `wp.customize.SelectiveRefresh.Partial`, capitalized unlike the namespace it lives in, and `Placement` named `wp.customize.selectiveRefresh` itself, so its methods were attached to the module rather than to the class.

Developed as subset of #10743.
Follow-up to r48110, r48650, r63454, r63455, r63456.

Props westonruter, afercia, grapplerulrich, mukesh27.
See #39671, #40831, #64662, #66033.


git-svn-id: https://develop.svn.wordpress.org/trunk@63457 602fd350-edb4-49c9-b593-d223f7449a82
markjaquith pushed a commit to markjaquith/WordPress that referenced this pull request Sep 3, 2026
Missing tags and descriptions are added, and the types are corrected, whether they named nothing resolvable (`Promise` for `JQuery.Promise<*>`), said too little (`Array` for `string[]`), or were narrower than what the code passes.

Both classes in selective-refresh.js lent their prototypes to the wrong name: `Partial` named `wp.customize.SelectiveRefresh.Partial`, capitalized unlike the namespace it lives in, and `Placement` named `wp.customize.selectiveRefresh` itself, so its methods were attached to the module rather than to the class.

Developed as subset of WordPress/wordpress-develop#10743.
Follow-up to r48110, r48650, r63454, r63455, r63456.

Props westonruter, afercia, grapplerulrich, mukesh27.
See #39671, #40831, #64662, #66033.

Built from https://develop.svn.wordpress.org/trunk@63457


git-svn-id: http://core.svn.wordpress.org/trunk@62637 1a063a9b-81f0-0310-95a4-ce76da25c4cd
wporg-sync pushed a commit that referenced this pull request Sep 4, 2026
Neither file documented a single method: only the classes themselves carried a docblock. Every method across the two now says what it does, in the third person, along with the arguments of the function wrapping each file.

One behavior is changed: `ChoiceListView.render()` returned nothing, unlike the sibling views beside it, so it now returns `this` as the Backbone convention expects. Nothing reads the value, since the view is only ever constructed, never chained, and `render` is otherwise reached as a `listenTo` handler, whose return is discarded.

Developed as subset of #10743.
Follow-up to r63454, r63455, r63456, r63457.

Props westonruter, afercia, grapplerulrich.
See #39671, #64662, #66033.
Fixes #40831.


git-svn-id: https://develop.svn.wordpress.org/trunk@63458 602fd350-edb4-49c9-b593-d223f7449a82
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

A commit was made that fixes the Trac ticket referenced in the description of this pull request.

SVN changeset: 63458
GitHub commit: f19cdb1

This PR will be closed, but please confirm the accuracy of this and reopen if there is more work to be done.

@github-actions github-actions Bot closed this Sep 4, 2026
markjaquith pushed a commit to markjaquith/WordPress that referenced this pull request Sep 4, 2026
Neither file documented a single method: only the classes themselves carried a docblock. Every method across the two now says what it does, in the third person, along with the arguments of the function wrapping each file.

One behavior is changed: `ChoiceListView.render()` returned nothing, unlike the sibling views beside it, so it now returns `this` as the Backbone convention expects. Nothing reads the value, since the view is only ever constructed, never chained, and `render` is otherwise reached as a `listenTo` handler, whose return is discarded.

Developed as subset of WordPress/wordpress-develop#10743.
Follow-up to r63454, r63455, r63456, r63457.

Props westonruter, afercia, grapplerulrich.
See #39671, #64662, #66033.
Fixes #40831.

Built from https://develop.svn.wordpress.org/trunk@63458


git-svn-id: http://core.svn.wordpress.org/trunk@62638 1a063a9b-81f0-0310-95a4-ce76da25c4cd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants