Re-enable the ESLint rules disabled for assets/js - #8230
Conversation
Turn on object-shorthand, prefer-const, no-var, no-else-return, no-lonely-if, no-useless-return, and prettier/prettier. All fixes are mechanical and behavior-preserving. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Turn on jsdoc/check-tag-names, require-param-type, require-returns-type, and require-returns-check. Add missing param/return types and descriptions, drop stale @return void tags, and allow the intentional @hook and @Usage custom tags via definedTags. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Turn on no-shadow, @wordpress/no-unused-vars-before-return, and @wordpress/i18n-text-domain. Rename shadowed variables, move declarations past early returns so they aren't assigned when unused, and correct the text domain on the AI upsell string from sensei-pro to sensei-lms. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Turn on no-alert. The two confirm() calls guarding destructive enrollment and progress changes are intentional, so keep them behind targeted eslint-disable-next-line comments rather than changing behavior. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Turn on eqeqeq. Most sites compare values already known to be strings or parseInt results, so they become === / !== directly. Where the loose compare intentionally coerced, preserve behavior for both string and numeric inputs: normalize the lesson quick-edit checkbox fields with an includes() membership check, and match the empty bulk action against the '0' placeholder string. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Turn on camelcase and rename local variables from snake_case to camelCase. Server-facing identifiers are preserved: object keys sent as $_POST/$_GET data, CSS selectors, HTML ids and localized globals (ajax_object, sensei_log_event, sensei_event_logging) are left unchanged, and properties:never keeps payload keys exempt. Renames were applied with scope-accurate binding analysis so every reference stays consistent. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
The updated assets/js/** ESLint enforcement likely breaks CI because at least one legacy script still violates rules that were previously disabled (e.g., no-alert/eqeqeq usage).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR restores a stricter ESLint rule set for assets/js/** (previously broadly disabled during the ESLint 10 flat-config migration) and applies mostly mechanical, behavior-preserving refactors to bring the code back into compliance. It also fixes a user-facing i18n issue where the “Generate quiz questions with AI” upsell label used the wrong text domain.
Changes:
- Re-enable ESLint enforcement for
assets/js/**and tighten JSDoc tag validation. - Mechanical JS cleanups across legacy jQuery/admin scripts (const/let, strict equality, shadowing fixes, object shorthand, etc.).
- Fix the upsell label’s text domain from
sensei-protosensei-lmsand add a changelog entry.
File summaries
| File | Description |
|---|---|
| eslint.config.js | Re-enables rule enforcement for assets/js/** and adjusts JSDoc tag-name validation / camelcase handling. |
| changelog/update-eslint-reenable-rules | Adds the patch-level changelog entry for the i18n fix. |
| assets/js/settings.js | JSDoc + strict equality and minor modernization. |
| assets/js/ranges.js | Modernizes variable declarations and object callback syntax. |
| assets/js/question-answer-tinymce-editor.js | Tightens JSDoc typing and strict equality for placeholder logic. |
| assets/js/modules-admin.js | Modernizes legacy admin script patterns (const/let, shorthand, etc.). |
| assets/js/learners-general.js | Refactors learner management actions for lint compliance (incl. confirm handling). |
| assets/js/learners-bulk-actions.js | Refactors bulk actions logic and preserves placeholder-action detection semantics. |
| assets/js/grading-general.js | Refactors grading/auto-grade logic for lint compliance and stricter comparisons. |
| assets/js/frontend/course-video/video-blocks-manager.js | Minor formatting + JSDoc tag normalization. |
| assets/js/file-upload-question-type.js | Minor formatting / readability adjustments. |
| assets/js/admin/settings/experimental-features.js | Minor modernization (const). |
| assets/js/admin/sensei-notice-dismiss.js | Improves JSDoc typing. |
| assets/js/admin/ordering.js | Minor modernization (let). |
| assets/js/admin/message-menu-fix.js | Minor modernization (const). |
| assets/js/admin/lesson-quick-edit.js | Preserves checkbox normalization behavior while eliminating loose equality. |
| assets/js/admin/lesson-bulk-edit.js | Minor modernization (const) in bulk edit handler. |
| assets/js/admin/lesson-ai.js | Fixes i18n text domain for the upsell label. |
| assets/js/admin/event-logging.js | Refactors event logging function signature / naming and const correctness. |
| assets/js/admin/course-general-sidebar.js | Avoids variable shadowing and tightens comparisons in sidebar controls. |
| assets/js/admin/course-edit.js | Refactors tracking callback naming / const usage. |
Review details
- Files reviewed: 21/21 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Rename the wp_localize_script handle from ajax_object to ajaxObject and update its sole JS reference, so the camelcase allow-list exemption is no longer needed. Drop it from the ESLint config. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
The sensei_event_logging → senseiEventLogging rename is incomplete and breaks existing runtime consumers (e.g. assets/shared/helpers/log-event.js).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
assets/js/learners-bulk-actions.js:211
$userIdand$dataNonceare plain string values, not jQuery objects; the$prefix is misleading here (especially next to$hiddenPosts, which is a jQuery object). Renaming improves readability and avoids confusion when maintaining this legacy jQuery code.
changelog/update-eslint-reenable-rules:4- This PR description indicates the changelog entry will be auto-generated by CI (“Automatically create a changelog entry”), but this PR also commits a
changelog/entry file. The repo convention is to do one or the other (not both), otherwise you can end up with duplicate entries or confusing review/CI behavior.
- Files reviewed: 22/22 changed files
- Comments generated: 1
- Review effort level: Lite
Rename the wp_localize_script handle from sensei_event_logging to senseiEventLogging and update its sole JS reference. The global is defined and consumed only within Sensei LMS (Sensei Pro references the same-named PHP filter hooks, not this JS global), so no allow-list exemption is needed. Only sensei_log_event, which Sensei Pro's frontend JS calls, remains exempt. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
5720f92 to
c90d2cd
Compare
Restore the ajax_object and sensei_event_logging wp_localize_script handles and JS references, and add them back to the camelcase allow-list. Renaming these window globals is a backward-incompatible change to undocumented surfaces third-party code could read, which is not worth it in a lint cleanup; exempting them (as with sensei_log_event) is the safe, consistent choice. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Proposed Changes
During the
@wordpress/scripts34 / ESLint 10 flat-config migration, a broad set of ESLint rules were switched off forassets/js/**to get CI green. This turns them back on and fixes the resulting violations, so the linter again catches real problems (unused variables, shadowing, loose equality) in this code.Almost all changes are mechanical and behavior-preserving. A few things are worth a reviewer's attention:
sensei-pro), so it was never translatable on a Sensei site. Corrected tosensei-lms.==comparisons that relied on type coercion were rewritten to keep the same result for both string and numeric inputs (lesson quick-edit checkbox normalization, empty bulk-action detection). Server-facing identifiers —$_POST/$_GETkeys, CSS selectors, HTML ids, and localized globals — were deliberately left unchanged during thecamelcaserenames.Testing Instructions
The riskiest edits are in untested legacy jQuery (grading, student management, lesson quick-edit) and the course editor sidebar. Verify these by hand:
Changelog entry
Changelog Entry Details
Significance
Type
Message
Fixed the text domain on the "Generate quiz questions with AI" upsell label so it can be translated.