Skip to content

Fix some improperly set JS vars with no var/let/const#2691

Merged
Crabcyborg merged 4 commits into
masterfrom
fix_vars_declared_improperly
Dec 29, 2025
Merged

Fix some improperly set JS vars with no var/let/const#2691
Crabcyborg merged 4 commits into
masterfrom
fix_vars_declared_improperly

Conversation

@Crabcyborg

@Crabcyborg Crabcyborg commented Dec 29, 2025

Copy link
Copy Markdown
Contributor

Detected using Sonar Scanner.

@coderabbitai

coderabbitai Bot commented Dec 29, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

Tightened variable scoping by converting implicitly declared or re-used variables to block-scoped const/let in two admin JS files; no API or behavioral changes observed. (49 words)

Changes

Cohort / File(s) Summary
Admin embed script
js/admin/embed.js
Replaced implicitly assigned doneButton with block-scoped const doneButton in two locations; no logic or event changes.
Admin UI script (multiple functions)
js/src/admin/admin.js
Introduced const/let for previously undeclared or re-used locals across many functions and loops (examples: $children, $msg, $unqDetail, rows, field, checkbox, loop indices). Scope tightened; behavior unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • shervElmi
  • lauramekaj1

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 5.88% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: fixing improperly declared JavaScript variables by adding var/let/const declarations.
Description check ✅ Passed The description references Sonar Scanner as the detection tool, which is related to the code quality improvements made in the changeset.
✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a331483 and 158d70a.

📒 Files selected for processing (2)
  • js/formidable_admin.js
  • js/src/admin/admin.js
🧰 Additional context used
🧬 Code graph analysis (1)
js/src/admin/admin.js (8)
js/formidable.js (1)
  • fieldId (2054-2054)
js/admin/embed.js (1)
  • id (490-490)
js/admin/dom.js (1)
  • id (145-145)
js/admin/applications.js (1)
  • id (240-240)
js/src/common/components/radio.js (1)
  • id (11-11)
js/admin/style.js (2)
  • checkbox (1100-1100)
  • i (707-707)
js/admin/settings.js (1)
  • data (90-90)
stripe/js/connect_settings.js (1)
  • data (76-80)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: PHP 8 tests in WP 6.9
  • GitHub Check: PHP 7.4 tests in WP 6.9
  • GitHub Check: Cypress
🔇 Additional comments (8)
js/src/admin/admin.js (8)

1130-1160: Scoped $children correctly in drag-over handler

Declaring $children as a local const inside handleDragOverYAxis avoids leaking it to the global scope and matches its read‑only usage; behavior of the function remains unchanged.


3387-3419: Local jQuery handles for validation boxes

Using const $msg and const $unqDetail local to toggleValidationBox and markUnique keeps these jQuery objects confined to their functions, with no reassignments; this is semantically correct and aligns with their usage.


4508-4511: Localized $fields in merge-layout click handler

The new const $fields = jQuery( '.frm-selected-field-group li.form-field' ); is only read and immediately passed to setupCustomLayoutOptions, so making it block‑scoped has no side effects and removes an implicit global.


5372-5468: Scoped rows and related locals in conditional-logic updaters

Both onOptionTextBlur and adjustConditionalLogicOptionOrders now declare rows (and other loop locals) within the function scope. This eliminates accidental sharing via implicit globals and does not change the logic that iterates .frm_logic_row elements.

Also applies to: 6093-6138


6417-6425: getChecked now uses local field/checkbox

Declaring field and checkbox as const inside getChecked correctly constrains them to the helper, avoids unintended globals, and keeps return semantics identical.


7929-7961: For-loop index i properly block-scoped in switchPostType

Updating for ( let i = 0; i < catRows.length; i++ ) prevents i from leaking into outer scope. No logic change, and the loop still operates over the same node list.


9406-9423: data object in trashTemplate correctly confined

Using const data = { ... } within trashTemplate guarantees the payload for postAjax is local and immutable for that call, eliminating an implicit global data without altering the AJAX behavior.


10872-10928: Inbox dismiss handler now uses local data

The click handler for .frm_inbox_dismiss now builds a const data object scoped to the handler before calling postAjax, which removes a potential global without affecting dismissal or redirect logic.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Crabcyborg Crabcyborg changed the title Fix some improperly set vars Fix some improperly set vars with no var/let/const Dec 29, 2025
@Crabcyborg Crabcyborg changed the title Fix some improperly set vars with no var/let/const Fix some improperly set JS vars with no var/let/const Dec 29, 2025
@Crabcyborg
Crabcyborg merged commit b4118f7 into master Dec 29, 2025
16 checks passed
@Crabcyborg
Crabcyborg deleted the fix_vars_declared_improperly branch December 29, 2025 21:20
stephywells pushed a commit that referenced this pull request Apr 4, 2026
Fix some improperly set JS vars with no var/let/const
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant