Skip to content

Enable the JSDoc rule valid-types #13486

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
afercia wants to merge 23 commits into from
Closed

Enable the JSDoc rule valid-types #13486

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
bbf4573
Enable JSDoc valid-types.
afercia Sep 11, 2026
3a7da74
Remove non-standard JSDoc liatens.
afercia Sep 20, 2026
5fc6bde
Fix valid types in inline-edit-tax.js file.
afercia Sep 20, 2026
c952955
Fix valid types in user-profile.js file.
afercia Sep 20, 2026
043dda7
Fix valid types in lists.js file.
afercia Sep 20, 2026
5ca7201
Fix valid types in wp api.js file.
afercia Sep 20, 2026
bad3cbe
Fix valid types in wp media editor.js file.
afercia Sep 20, 2026
d978758
Remove docblock for local variable in theme.js file.
afercia Sep 20, 2026
da42376
Fix valid types in widgets media.js file.
afercia Sep 20, 2026
dc86b1f
Fix valid types in media models attachment.js file.
afercia Sep 20, 2026
a367980
Fix valid types in media models attachments.js file.
afercia Sep 20, 2026
9df8a69
Remove docblock for local variable in media models query.js file.
afercia Sep 20, 2026
f8c9155
Remove invalid fires notations in media models selection.js file.
afercia Sep 20, 2026
550dedd
Remove invalid fires notations in media views attachment details.js f…
afercia Sep 20, 2026
5303c3c
Fix valid types in media views attachments.js file.
afercia Sep 20, 2026
30d269c
Fix valid types in media views frame manage.js file.
afercia Sep 20, 2026
d93c359
Fix valid types in media views frame select.js file.
afercia Sep 20, 2026
f7cfcde
Remove invalid event notation in media views frame.js file.
afercia Sep 20, 2026
8c053ec
Remove invalid fires notations in media views media-details.js file.
afercia Sep 20, 2026
06b5211
Fix valid types in media views media-frame.js file.
afercia Sep 20, 2026
1201c48
Fix wp api.js file.
afercia Sep 20, 2026
373de73
Use prose description instead of changing listens notations meaning.
afercia Sep 20, 2026
43e5ea5
Improve wp api.hs more function return notation.
afercia Sep 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .eslintrc-jsdoc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ module.exports = [
'jsdoc/require-returns-check': 'error',
'jsdoc/require-returns-description': 'error',
'jsdoc/require-returns-type': 'error',
'jsdoc/valid-types': 'error',
},
},
];
12 changes: 0 additions & 12 deletions src/js/_enqueues/admin/comment.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ jQuery( function($) {
/**
* Adds event that opens the time stamp form if the form is hidden.
*
* @listens $edittimestamp:click

@westonruter westonruter Sep 21, 2026

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.

Why did these @listens tags need to be removed? They seem valid: https://jsdoc.app/tags-listens

@afercia afercia Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Same: JSDoc doesn't validate them. It doesn't understand this 'path'. Also, not sure what value these @listens tag may add because the function is self-explanatory.... when I read:
$edittimestamp.on( 'click', ....
I already see what I need to know.
That said, if you can find a way to make JSDoc understand these @listens and not report a violation, I'm all for it.

*
* @param {Event} event The event object.
* @return {void}
*/
Expand All @@ -43,8 +41,6 @@ jQuery( function($) {
/**
* Resets the time stamp values when the cancel button is clicked.
*
* @listens .cancel-timestamp:click
*
* @param {Event} event The event object.
* @return {void}
*/
Expand All @@ -65,8 +61,6 @@ jQuery( function($) {
/**
* Sets the time stamp values when the ok button is clicked.
*
* @listens .save-timestamp:click
*
* @param {Event} event The event object.
* @return {void}
*/
Expand Down Expand Up @@ -110,8 +104,6 @@ jQuery( function($) {
*
* @since 7.1.0
*
* @listens $editcommentparent:click
*
* @param {Event} event The event object.
* @return {void}
*/
Expand All @@ -131,8 +123,6 @@ jQuery( function($) {
*
* @since 7.1.0
*
* @listens .cancel-comment-parent:click
*
* @param {Event} event The event object.
* @return {void}
*/
Expand All @@ -150,8 +140,6 @@ jQuery( function($) {
*
* @since 7.1.0
*
* @listens .save-comment-parent:click
*
* @param {Event} event The event object.
* @return {void}
*/
Expand Down
10 changes: 5 additions & 5 deletions src/js/_enqueues/admin/inline-edit-tax.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ window.inlineEditTax = {
*
* @since 2.7.0
*
* @this inlineEditTax
* @this {inlineEditTax}
* @memberof inlineEditTax
* @return {void}
*/
Expand Down Expand Up @@ -93,7 +93,7 @@ window.inlineEditTax = {
*
* @since 2.7.0
*
* @this inlineEditTax
* @this {inlineEditTax}
* @memberof inlineEditTax
*
* @param {HTMLElement} el An element within the table row or the table row
Expand All @@ -111,7 +111,7 @@ window.inlineEditTax = {
*
* @since 2.7.0
*
* @this inlineEditTax
* @this {inlineEditTax}
* @memberof inlineEditTax
*
* @param {string|HTMLElement} id The ID of the term we want to quick edit or an
Expand Down Expand Up @@ -158,7 +158,7 @@ window.inlineEditTax = {
*
* @since 2.7.0
*
* @this inlineEditTax
* @this {inlineEditTax}
* @memberof inlineEditTax
*
* @param {string|HTMLElement} id The ID of the term we want to quick edit or an
Expand Down Expand Up @@ -255,7 +255,7 @@ window.inlineEditTax = {
*
* @since 2.7.0
*
* @this inlineEditTax
* @this {inlineEditTax}
* @memberof inlineEditTax
* @return {void}
*/
Expand Down
8 changes: 4 additions & 4 deletions src/js/_enqueues/admin/user-profile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,10 +186,10 @@
/**
* Helper function to insert an inline notice of success or failure.
*
* @param {jQuery Object} $this The button element: the message will be inserted
* above this button
* @param {boolean} success Whether the message is a success message.
* @param {string} message The message to insert.
* @param {jQuery} $this The button element: the message will be inserted
* above this button
* @param {boolean} success Whether the message is a success message.
* @param {string} message The message to insert.
*/
function addInlineNotice( $this, success, message ) {
var resultDiv = $( '<div />', {
Expand Down
46 changes: 23 additions & 23 deletions src/js/_enqueues/lib/lists.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -812,31 +812,31 @@ wpList = {
/**
* Initializes wpList object.
*
* @param {Object} settings The settings for the wpList instance.
* @param {string} settings.url URL for ajax calls. Default: ajaxurl.
* @param {string} settings.type The HTTP method to use for Ajax requests. Default: 'POST'.
* @param {string} settings.response ID of the element the parsed ajax response will be stored in.
* Default: 'ajax-response'.
* @param {Object} settings The settings for the wpList instance.
* @param {string} settings.url URL for ajax calls. Default: ajaxurl.
* @param {string} settings.type The HTTP method to use for Ajax requests. Default: 'POST'.
* @param {string} settings.response ID of the element the parsed ajax response will be stored in.
* Default: 'ajax-response'.
*
* @param {string} settings.what Default: ''.
* @param {string} settings.alt CSS class name for alternate styling. Default: 'alternate'.
* @param {number} settings.altOffset Offset to start alternate styling from. Default: 0.
* @param {string} settings.addColor Hex code or 'none' to disable animation. Default: '#ffff33'.
* @param {string} settings.delColor Hex code or 'none' to disable animation. Default: '#faafaa'.
* @param {string} settings.dimAddColor Hex code or 'none' to disable animation. Default: '#ffff33'.
* @param {string} settings.dimDelColor Hex code or 'none' to disable animation. Default: '#ff3333'.
* @param {string} settings.what Default: ''.
* @param {string} settings.alt CSS class name for alternate styling. Default: 'alternate'.
* @param {number} settings.altOffset Offset to start alternate styling from. Default: 0.
* @param {string} settings.addColor Hex code or 'none' to disable animation. Default: '#ffff33'.
* @param {string} settings.delColor Hex code or 'none' to disable animation. Default: '#faafaa'.
* @param {string} settings.dimAddColor Hex code or 'none' to disable animation. Default: '#ffff33'.
* @param {string} settings.dimDelColor Hex code or 'none' to disable animation. Default: '#ff3333'.
*
* @param {wpList~confirm} settings.confirm Callback that's run before a request is made. Default: null.
* @param {wpList~addBefore} settings.addBefore Callback that's run before an item gets added to the list.
* Default: null.
* @param {wpList~addAfter} settings.addAfter Callback that's run after an item got added to the list.
* Default: null.
* @param {wpList~delBefore} settings.delBefore Callback that's run before an item gets deleted from the list.
* Default: null.
* @param {wpList~delAfter} settings.delAfter Callback that's run after an item got deleted from the list.
* Default: null.
* @param {wpList~dimBefore} settings.dimBefore Callback that's run before an item gets dim'd. Default: null.
* @param {wpList~dimAfter} settings.dimAfter Callback that's run after an item got dim'd. Default: null.
Comment on lines -829 to -839

@westonruter westonruter Sep 21, 2026

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.

I'm not familiar with this ~ syntax. It does seem unnecessary regardless. However, with TypeScript we could potentially re-introduce some flavor of that.

@afercia afercia Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

JSDoc doesn't validate them. Not sure that ~ syntax is still valid. JSDoc can't understand it anyways. After all these are all callback functions. We will see when you will extend TypeScript to this file.

* @param {Function} settings.confirm Callback that's run before a request is made. Default: null.
* @param {Function} settings.addBefore Callback that's run before an item gets added to the list.
* Default: null.
* @param {Function} settings.addAfter Callback that's run after an item got added to the list.
* Default: null.
* @param {Function} settings.delBefore Callback that's run before an item gets deleted from the list.
* Default: null.
* @param {Function} settings.delAfter Callback that's run after an item got deleted from the list.
* Default: null.
* @param {Function} settings.dimBefore Callback that's run before an item gets dim'd. Default: null.
* @param {Function} settings.dimAfter Callback that's run after an item got dim'd. Default: null.
* @return {$.fn} wpList API function.
*/
$.fn.wpList = function( settings ) {
Expand Down
19 changes: 10 additions & 9 deletions src/js/_enqueues/wp/api.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
* Determine model based on API route.
*
* @param {string} route The API route.
* @return {Backbone Model} The model found at given route. Undefined if not found.
* @return {Backbone.Model|undefined} The model found at given route. Undefined if not found.
*/
wp.api.getModelByRoute = function( route ) {
return _.find( wp.api.models, function( model ) {
Expand All @@ -72,7 +72,7 @@
* Determine collection based on API route.
*
* @param {string} route The API route.
* @return {Backbone Model} The collection found at given route. Undefined if not found.
* @return {Backbone.Collection|undefined} The collection found at given route. Undefined if not found.
*/
wp.api.getCollectionByRoute = function( route ) {
return _.find( wp.api.collections, function( collection ) {
Expand Down Expand Up @@ -311,7 +311,7 @@
/**
* Add mixins and helpers to models depending on their defaults.
*
* @param {Backbone Model} model The model to attach helpers and mixins to.
* @param {Backbone.Model} model The model to attach helpers and mixins to.
* @param {string} modelClassName The classname of the constructed model.
* @param {Object} loadingObjects An object containing the models and collections we are building.
* @return {undefined} No return value.
Expand Down Expand Up @@ -868,9 +868,9 @@
/**
* Set nonce header before every Backbone sync.
*
* @param {string} method The CRUD method ("create", "read", "update", or "delete") to be performed.
* @param {Backbone.Model} model The model to be synced.
* @param {{beforeSend}, *} options Additional options for the sync.
* @param {string} method The CRUD method ("create", "read", "update", or "delete") to be performed.
* @param {Backbone.Model} model The model to be synced.
* @param {Object} options Additional options for the sync.

@westonruter westonruter Sep 21, 2026

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.

This is now less-specific than before. It seems like it should have been something like this:

Suggested change
* @param {Object} options Additional options for the sync.
* @param {Object} options Additional options for the sync.
* @param {Function} [options.beforeSend] Callback to run before sending.
* @param {Function} [options.success] Callback executed on a successful request.
* @param {Function} [options.error] Callback executed on a failed request.

@afercia afercia Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't see options.success or options.error here, while I do see options.complete. I will add it.

@westonruter westonruter Sep 21, 2026

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.

They are documented in Backbone's options: https://backbonejs.org/#Sync

@afercia afercia Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I see and all other jQuery request options
Does that mean we should really document ALL those options? I'd say no, and maybe we should just keep the main options object.

@westonruter westonruter Sep 21, 2026

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.

Probably, not, no. Let's just do beforeSend.

* @return {*}.
*/
sync: function( method, model, options ) {
Expand Down Expand Up @@ -1033,7 +1033,7 @@
*
* @param {string} method The CRUD method ("create", "read", "update", or "delete") to be performed.
* @param {Backbone.Model} model The model to be synced.
* @param {{success}, *} options Additional options for the sync.
* @param {Object} options Additional options for the sync.

@westonruter westonruter Sep 21, 2026

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.

Ditto above.

* @return {*}.
*/
sync: function( method, model, options ) {
Expand Down Expand Up @@ -1108,8 +1108,9 @@
/**
* Fetches the next page of objects if a new page exists.
*
* @param {data: {page}} options An object containing the page number to fetch. If not provided, the next page will be fetched.
* @return {*}.
* @param {Object} [options] An object containing the page number to fetch. If not provided, the next page will be fetched.
* @param {number} [options.data.page] The page number to fetch.
* @return {JQuery.Promise} A promise that resolves when the fetch operation completes.
*/
more: function( options ) {
options = options || {};
Expand Down
26 changes: 13 additions & 13 deletions src/js/_enqueues/wp/media/editor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,7 @@
* @param {string} content Content that is searched for possible
* shortcode markup matching the passed tag name,
*
* @this wp.media.{prop}
* @this {wp.media.view.MediaFrame.Select}
*
* @return {void|wp.media.view.MediaFrame.Select} A media workflow.
*/
Expand Down Expand Up @@ -649,7 +649,7 @@
/**
* The Featured Image workflow
*
* @this wp.media.featuredImage
* @this {wp.media.featuredImage}
*
* @return {wp.media.view.MediaFrame.Select} A media workflow.
*/
Expand All @@ -666,7 +666,7 @@

this._frame.on( 'toolbar:create:featured-image', function( toolbar ) {
/**
* @this wp.media.view.MediaFrame.Select
* @this {wp.media.view.MediaFrame.Select}
*/
this.createSelectToolbar( toolbar, {
text: wp.media.view.l10n.setFeaturedImage
Expand All @@ -691,7 +691,7 @@
* 'select' callback for Featured Image workflow, triggered when
* the 'Set Featured Image' button is clicked in the media modal.
*
* @this wp.media.controller.FeaturedImage
* @this {wp.media.controller.FeaturedImage}
*/
select: function() {
var selection = this.get('selection').single();
Expand Down Expand Up @@ -789,7 +789,7 @@
* @param {string} id A slug used to identify the workflow.
* @param {Object} [options={}] The options for the media workflow.
*
* @this wp.media.editor
* @this {wp.media.editor}
*
* @return {wp.media.view.MediaFrame.Select} A media workflow.
*/
Expand Down Expand Up @@ -820,7 +820,7 @@
$.when.apply( $, selection.map( function( attachment ) {
var display = state.display( attachment ).toJSON();
/**
* @this wp.media.editor
* @this {wp.media.editor}
*/
return this.send.attachment( display, attachment.toJSON() );
}, this ) ).done( function() {
Expand All @@ -830,28 +830,28 @@

workflow.state('gallery-edit').on( 'update', function( selection ) {
/**
* @this wp.media.editor
* @this {wp.media.editor}
*/
this.insert( wp.media.gallery.shortcode( selection ).string() );
}, this );

workflow.state('playlist-edit').on( 'update', function( selection ) {
/**
* @this wp.media.editor
* @this {wp.media.editor}
*/
this.insert( wp.media.playlist.shortcode( selection ).string() );
}, this );

workflow.state('video-playlist-edit').on( 'update', function( selection ) {
/**
* @this wp.media.editor
* @this {wp.media.editor}
*/
this.insert( wp.media.playlist.shortcode( selection ).string() );
}, this );

workflow.state('embed').on( 'select', function() {
/**
* @this wp.media.editor
* @this {wp.media.editor}
*/
var state = workflow.state(),
type = state.get('type'),
Expand Down Expand Up @@ -920,7 +920,7 @@
*
* @param {string} id A slug used to identify the workflow.
*
* @this wp.media.editor
* @this {wp.media.editor}
*
* @return {wp.media.view.MediaFrame} A media workflow.
*/
Expand All @@ -933,7 +933,7 @@
*
* @param {string} id A slug used to identify the workflow.
*
* @this wp.media.editor
* @this {wp.media.editor}
*/
remove: function( id ) {
id = this.id( id );
Expand Down Expand Up @@ -1020,7 +1020,7 @@
* @param {string} [id=undefined] Optional. A slug used to identify the workflow.
* @param {Object} [options={}] The options for the media workflow.
*
* @this wp.media.editor
* @this {wp.media.editor}
*
* @return {wp.media.view.MediaFrame} A media workflow.
*/
Expand Down
4 changes: 0 additions & 4 deletions src/js/_enqueues/wp/theme.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,10 +275,6 @@ themes.Collection = Backbone.Collection.extend({
* which triggers events of `query:success` or `query:fail`.
*/
query: function( request ) {
/**
* @static
* @type Array
*/
Comment on lines -278 to -281

@westonruter westonruter Sep 21, 2026

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.

Curious why this needs to be removed?

@afercia afercia Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Again, JSDoc doesn't validate this. Also, is it a static? Looks like just a local vari to me.

var queries = this.queries,
self = this,
query, isPaginated, count;
Expand Down
2 changes: 1 addition & 1 deletion src/js/_enqueues/wp/widgets/media.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ wp.mediaWidgets = ( function( $ ) {
*
* @ignore
*
* @fires wp.media.controller.State#insert()
* @fires wp.media.controller.State#insert
* @return {void}
*/
click: function onClick() {
Expand Down
2 changes: 1 addition & 1 deletion src/js/media/models/attachment.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ Attachment = Backbone.Model.extend(/** @lends wp.media.model.Attachment.prototyp
* @param {Object} data The properties to be saved.
* @param {Object} options Sync options. e.g. patch, wait, success, error.
*
* @this Backbone.Model
* @this {Backbone.Model}
*
* @return {jQuery.Promise} A jQuery Promise that is resolved or rejected based on the success of the sync operation.
*/
Expand Down
Loading
Loading