Fix ESLint: restore @wordpress/eslint-plugin rules compatibility - #2980
Merged
Conversation
Contributor
Author
|
@Crabcyborg, I'm aware that this PR doesn't fix the 387 ESLint errors that are now being detected. I'll address them separately by opening a PR that adds new Formidable Forms ESLint rules, with this PR as the base branch. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughLarge-scale JSDoc normalization converting plural Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
js/src/admin/components/dependent-updater-component.js (1)
49-54:⚠️ Potential issue | 🟡 MinorPotential crash when
propagateInputsis empty.If none of the names in
willChangeDatamatch any DOM input,propagateInputswill be an empty array. Line 53 then accesses index0unconditionally, throwing aTypeError: Cannot read properties of undefined (reading 'dispatchEvent').🛡️ Proposed fix to guard the dispatch call
updateAllDependentElements( value ) { + if ( ! this.data.propagateInputs.length ) { + return; + } this.data.propagateInputs.forEach( input => { input.value = value; } ); this.data.propagateInputs[ 0 ].dispatchEvent( this.data.changeEvent ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/src/admin/components/dependent-updater-component.js` around lines 49 - 54, The method updateAllDependentElements currently assumes this.data.propagateInputs has at least one element and unconditionally calls this.data.propagateInputs[0].dispatchEvent(this.data.changeEvent); to avoid the TypeError when propagateInputs is empty, add a guard: check that this.data.propagateInputs exists and has length > 0 before accessing index 0 (or alternatively iterate and dispatch on each input). Update updateAllDependentElements to only call dispatchEvent when an input is present, referencing this.data.propagateInputs and this.data.changeEvent.
🧹 Nitpick comments (6)
js/src/form-templates/elements/applicationTemplatesElement.js (1)
26-30: Align function description with actual return behavior.
createApplicationTemplatesis documented as “Create and return…”, but it mutates module state and returns nothing. Consider adjusting the sentence to avoid “return”.Suggested doc-only tweak
- * Create and return the application templates HTML element. + * Create the application templates HTML element.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/src/form-templates/elements/applicationTemplatesElement.js` around lines 26 - 30, Update the JSDoc for createApplicationTemplates to reflect that it mutates module state and does not return a value: change the description from "Create and return the application templates HTML element." to something like "Create and populate the application templates HTML element." and keep or ensure the `@return` is {void}; reference the function name createApplicationTemplates and the module-level mutation to make the intent clear.js/src/core/ui/counter.js (1)
9-11: AligncounterJSDoc with nullable return behavior.
counter(...)returnsnullfor invalid elements, so the return type should includenull.Proposed doc fix
- * `@throws` {Error} When element is not found or invalid - * `@return` {HTMLElement} The updated element for method chaining + * `@return` {HTMLElement|null} The updated element for method chaining, or null when target is invalid🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/src/core/ui/counter.js` around lines 9 - 11, Update the JSDoc for the counter function to reflect that it can return null for invalid elements: change the `@return` annotation from {HTMLElement} to {HTMLElement|null} and, if the implementation no longer throws for invalid elements, update or remove the `@throws` {Error} line to match the actual behavior in function counter so the documentation and implementation remain consistent.js/admin/dom.js (1)
458-459: FixgetCookiedescription/type mismatch.The annotation says
string|null, but the description still saysundefinedfor missing cookies.Proposed doc fix
- * `@return` {string|null} The value of the cookie, or undefined if the cookie does not exist. + * `@return` {string|null} The value of the cookie, or null if the cookie does not exist.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/admin/dom.js` around lines 458 - 459, Update the JSDoc for getCookie so the return type and description match: change the description text that currently says "undefined" for missing cookies to state "null" (or alternatively change the `@return` to `string|undefined` if you prefer undefined semantics), and ensure the `@return` line reads `@return {string|null} The value of the cookie, or null if the cookie does not exist.` so the comment and type annotation are consistent with the getCookie implementation.js/src/web-components/frm-typography-component/frm-typography-component.js (1)
212-213: Remove contradictory wording fromafterViewInitreturn doc.
@return {void}should not describe a returned unit value.Proposed doc fix
- * `@return` {void} - The unit value. + * `@return` {void}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/src/web-components/frm-typography-component/frm-typography-component.js` around lines 212 - 213, The JSDoc for the afterViewInit method contains contradictory wording: the `@return` {void} tag should not describe a returned "unit value"; edit the JSDoc block for afterViewInit to remove the phrase "The unit value." so the `@return` {void} has no misleading description (or remove the `@return` tag entirely if you prefer no return tag for void methods).js/src/onboarding-wizard/dataUtils/setupUsageData.js (1)
30-31: AlignprocessDataForStepJSDoc with actual return value.Current code can return
undefined, but the JSDoc saysFormData|null. Please either initializeformDatatonullor update the type to includeundefined.Option A (keep current logic, fix doc)
- * `@return` {FormData|null} The FormData to be submitted for the step, or null if there's no data. + * `@return` {FormData|undefined} The FormData to be submitted for the step, or undefined if there's no data.Option B (keep doc intent, fix code)
function processDataForStep( processedStep, nextStepName ) { - let formData; + let formData = null;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/src/onboarding-wizard/dataUtils/setupUsageData.js` around lines 30 - 31, The JSDoc for processDataForStep claims it returns FormData|null but the function can return undefined; fix this by making the function follow the documented contract: initialize the local variable formData to null (instead of leaving it undefined) in processDataForStep and ensure every code path explicitly returns either formData (a FormData instance) or null; update any early returns to return null rather than leaving an implicit undefined so the runtime behavior matches the JSDoc.js/src/web-components/frm-tab-navigator-component/frm-tab-navigator-component.js (1)
14-14: Fix JSDoc return types to match actual return values.Several updated
@returnannotations don’t match implementation: multiple methods returnElement(notstring), and two methods are nullable (initView,getTabUnderline).Proposed JSDoc corrections
- * `@return` {Element} - The wrapper element. + * `@return` {Element|null} - The wrapper element.- * `@return` {string} - The tab delimiter. + * `@return` {Element} - The tab delimiter.- * `@return` {string} - The tab headings. + * `@return` {Element} - The tab headings.- * `@return` {string} - The tab container. + * `@return` {Element} - The tab container.- * `@return` {string} - The tab heading. + * `@return` {Element} - The tab heading.- * `@return` {string} - The tab container. + * `@return` {Element} - The tab container.- * `@return` {Element} - The tab underline. + * `@return` {Element|null} - The tab underline.Also applies to: 55-56, 72-73, 91-92, 113-114, 128-129, 146-146
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/src/web-components/frm-tab-navigator-component/frm-tab-navigator-component.js` at line 14, Update the incorrect JSDoc `@return` annotations in frm-tab-navigator-component.js so they match actual return values: change return types that are currently "string" to "Element" for methods that return DOM elements (e.g., the wrapper getter, tab list/item/anchor getters such as getWrapperElement, getTabList, getTabItems, getTabAnchors, and any similar accessors), and mark initView and getTabUnderline as nullable (e.g., `@return` {Element|null}) since they can return null; ensure getActiveTabId (or any method that truly returns a string) keeps `@return` {string}. Adjust only the `@return` types in the JSDoc blocks for the listed functions to reflect Element vs string and nullable Element where applicable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@js/src/admin/admin.js`:
- Around line 8752-8754: The code assumes contextualShortcodes and each queried
element exist before using them: guard access to contextualShortcodes and the
element found by document.querySelector before calling matches/toggling. In the
block where shouldShowShortcodes is computed and the loop over
contextualShortcodes runs, first check that contextualShortcodes is an array/has
length and that selector is defined, then when building shortcodeLi from
document.querySelector(...)?closest('li') ensure you test the query result (or
shortcodeLi) is not null before using it; apply these guards around the usage of
shouldShowShortcodes, the for (const shortcode of contextualShortcodes) loop,
and any operations on shortcodeLi to avoid runtime exceptions.
In `@js/src/core/utils/async.js`:
- Line 10: The current addToRequestQueue implementation calls .catch(task) which
re-invokes the task on rejection; update addToRequestQueue (and any usage of
lastPromise) so failed tasks are not retried automatically: change the rejection
handler to swallow or log the error instead of re-running task (e.g., .catch(err
=> { /* log err */ }) ), preserving the promise chain by returning a resolved
value or the error as appropriate so subsequent queued tasks still run and do
not double-execute non-idempotent operations.
---
Outside diff comments:
In `@js/src/admin/components/dependent-updater-component.js`:
- Around line 49-54: The method updateAllDependentElements currently assumes
this.data.propagateInputs has at least one element and unconditionally calls
this.data.propagateInputs[0].dispatchEvent(this.data.changeEvent); to avoid the
TypeError when propagateInputs is empty, add a guard: check that
this.data.propagateInputs exists and has length > 0 before accessing index 0 (or
alternatively iterate and dispatch on each input). Update
updateAllDependentElements to only call dispatchEvent when an input is present,
referencing this.data.propagateInputs and this.data.changeEvent.
---
Nitpick comments:
In `@js/admin/dom.js`:
- Around line 458-459: Update the JSDoc for getCookie so the return type and
description match: change the description text that currently says "undefined"
for missing cookies to state "null" (or alternatively change the `@return` to
`string|undefined` if you prefer undefined semantics), and ensure the `@return`
line reads `@return {string|null} The value of the cookie, or null if the cookie
does not exist.` so the comment and type annotation are consistent with the
getCookie implementation.
In `@js/src/core/ui/counter.js`:
- Around line 9-11: Update the JSDoc for the counter function to reflect that it
can return null for invalid elements: change the `@return` annotation from
{HTMLElement} to {HTMLElement|null} and, if the implementation no longer throws
for invalid elements, update or remove the `@throws` {Error} line to match the
actual behavior in function counter so the documentation and implementation
remain consistent.
In `@js/src/form-templates/elements/applicationTemplatesElement.js`:
- Around line 26-30: Update the JSDoc for createApplicationTemplates to reflect
that it mutates module state and does not return a value: change the description
from "Create and return the application templates HTML element." to something
like "Create and populate the application templates HTML element." and keep or
ensure the `@return` is {void}; reference the function name
createApplicationTemplates and the module-level mutation to make the intent
clear.
In `@js/src/onboarding-wizard/dataUtils/setupUsageData.js`:
- Around line 30-31: The JSDoc for processDataForStep claims it returns
FormData|null but the function can return undefined; fix this by making the
function follow the documented contract: initialize the local variable formData
to null (instead of leaving it undefined) in processDataForStep and ensure every
code path explicitly returns either formData (a FormData instance) or null;
update any early returns to return null rather than leaving an implicit
undefined so the runtime behavior matches the JSDoc.
In
`@js/src/web-components/frm-tab-navigator-component/frm-tab-navigator-component.js`:
- Line 14: Update the incorrect JSDoc `@return` annotations in
frm-tab-navigator-component.js so they match actual return values: change return
types that are currently "string" to "Element" for methods that return DOM
elements (e.g., the wrapper getter, tab list/item/anchor getters such as
getWrapperElement, getTabList, getTabItems, getTabAnchors, and any similar
accessors), and mark initView and getTabUnderline as nullable (e.g., `@return`
{Element|null}) since they can return null; ensure getActiveTabId (or any method
that truly returns a string) keeps `@return` {string}. Adjust only the `@return`
types in the JSDoc blocks for the listed functions to reflect Element vs string
and nullable Element where applicable.
In `@js/src/web-components/frm-typography-component/frm-typography-component.js`:
- Around line 212-213: The JSDoc for the afterViewInit method contains
contradictory wording: the `@return` {void} tag should not describe a returned
"unit value"; edit the JSDoc block for afterViewInit to remove the phrase "The
unit value." so the `@return` {void} has no misleading description (or remove the
`@return` tag entirely if you prefer no return tag for void methods).
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (96)
eslint.config.mjsjs/admin/dom.jsjs/admin/embed.jsjs/admin/style.jsjs/formidable.jsjs/packages/floating-links/s11-floating-links.jsjs/plugin-search.jsjs/src/addons-page/addons/categorizeAddons.jsjs/src/addons-page/events/addonToggleListener.jsjs/src/addons-page/events/index.jsjs/src/addons-page/events/searchListener.jsjs/src/addons-page/ui/searchState.jsjs/src/addons-page/ui/setupInitialView.jsjs/src/addons-page/ui/showEmptyState.jsjs/src/addons-page/ui/showSelectedCategory.jsjs/src/admin/addon-state.jsjs/src/admin/admin.jsjs/src/admin/components/dependent-updater-component.jsjs/src/admin/styles.jsjs/src/api/class-addon-api.jsjs/src/common/components/icon.jsjs/src/common/utilities/values.jsjs/src/core/events/optionBoxListener.jsjs/src/core/factory/createPageElements.jsjs/src/core/factory/createPageState.jsjs/src/core/page-skeleton/elements/emptyStateElement.jsjs/src/core/page-skeleton/events/categoryListener.jsjs/src/core/page-skeleton/events/index.jsjs/src/core/ui/addProgressToCardBoxes.jsjs/src/core/ui/counter.jsjs/src/core/utils/async.jsjs/src/core/utils/error.jsjs/src/core/utils/url.jsjs/src/core/utils/validation.jsjs/src/core/utils/visibility.jsjs/src/form-templates/elements/applicationTemplatesElement.jsjs/src/form-templates/events/applicationTemplateListener.jsjs/src/form-templates/events/createFormButtonListener.jsjs/src/form-templates/events/createTemplateListeners.jsjs/src/form-templates/events/favoriteButtonListener.jsjs/src/form-templates/events/getFreeTemplatesListener.jsjs/src/form-templates/events/index.jsjs/src/form-templates/events/searchListener.jsjs/src/form-templates/events/useTemplateButtonListener.jsjs/src/form-templates/initializeFormTemplates.jsjs/src/form-templates/templates/applicationTemplates.jsjs/src/form-templates/templates/categorizeTemplates.jsjs/src/form-templates/ui/initializeModal.jsjs/src/form-templates/ui/pageTitle.jsjs/src/form-templates/ui/searchState.jsjs/src/form-templates/ui/setupInitialView.jsjs/src/form-templates/ui/showEmptyState.jsjs/src/form-templates/ui/showError.jsjs/src/form-templates/ui/showHeaderCancelButton.jsjs/src/form-templates/ui/showModal.jsjs/src/form-templates/ui/showSelectedCategory.jsjs/src/form-templates/utils/validation.jsjs/src/onboarding-wizard/dataUtils/setupUsageData.jsjs/src/onboarding-wizard/events/backButtonListener.jsjs/src/onboarding-wizard/events/consentTrackingButtonListener.jsjs/src/onboarding-wizard/events/index.jsjs/src/onboarding-wizard/events/installAddonsButtonListener.jsjs/src/onboarding-wizard/events/skipStepButtonListener.jsjs/src/onboarding-wizard/initializeOnboardingWizard.jsjs/src/onboarding-wizard/ui/rootline.jsjs/src/onboarding-wizard/ui/setupInitialView.jsjs/src/onboarding-wizard/utils/navigateToStep.jsjs/src/settings-components/components/radio-component.jsjs/src/settings-components/components/slider-component.jsjs/src/settings-components/components/toggle-group/toggle-group.jsjs/src/settings-components/components/token-input/event-handlers.jsjs/src/settings-components/components/token-input/proxy-input-style.jsjs/src/settings-components/components/token-input/token-actions.jsjs/src/settings-components/components/token-input/token-elements.jsjs/src/settings-components/components/token-input/token-input.jsjs/src/settings-components/components/unit-input.jsjs/src/web-components/frm-colorpicker-component/frm-colorpicker-component.jsjs/src/web-components/frm-dropdown-component/frm-dropdown-component.jsjs/src/web-components/frm-range-slider-component/frm-range-slider-component.jsjs/src/web-components/frm-tab-navigator-component/frm-tab-navigator-component.jsjs/src/web-components/frm-typography-component/frm-typography-component.jsjs/src/web-components/frm-web-component.jsjs/src/welcome-tour/elements/beginTourModalElement.jsjs/src/welcome-tour/events/checklistEvents.jsjs/src/welcome-tour/events/dismissEvents.jsjs/src/welcome-tour/events/index.jsjs/src/welcome-tour/events/stylerUpdateButtonEvents.jsjs/src/welcome-tour/ui/checklist.jsjs/src/welcome-tour/ui/modal.jsjs/src/welcome-tour/ui/spotlight.jsjs/src/welcome-tour/utils/markStepAsCompleted.jsjs/src/welcome-tour/utils/pageDetection.jspackage.jsonsquare/js/frontend.jsstripe/js/frmstrp.jstests/cypress/e2e/Forms/formPageDataValidation.cy.js
💤 Files with no reviewable changes (1)
- tests/cypress/e2e/Forms/formPageDataValidation.cy.js
 |
|
Overall Grade Focus Area: Reliability |
Security  Reliability  Complexity  Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| PHP |  |
Feb 25, 2026 2:25p.m. | Review ↗ |
| JavaScript |  |
Feb 25, 2026 2:25p.m. | Review ↗ |
![deepsource-io[bot]](https://avatars.githubusercontent.com/in/16372?s=60&v=4){kind=small}
Contributor
Thanks @shervElmi. I don't want to merge this with errors, so I added some overrides for now to ignore the errors in the problematic files. We can remove those when the issues are actually fixed. |
stephywells
pushed a commit
that referenced
this pull request
Apr 4, 2026
…compatibility Fix ESLint: restore @wordpress/eslint-plugin rules compatibility
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
@wordpress/eslint-pluginhasn't migrated to ESLint 9 yet, and since Formidable Forms uses ESLint 9, all rules from@wordpress/eslint-pluginand some other ESLint plugins were excluded from the ESLint configuration. This preventednpm run lint:fixfrom automatically fixing violations of these rules.Summary by CodeRabbit
Documentation
Chores