Skip to content

Add unicorn eslint rules#2724

Merged
Crabcyborg merged 4 commits into
masterfrom
add_unicorn_eslint_rules
Jan 6, 2026
Merged

Add unicorn eslint rules#2724
Crabcyborg merged 4 commits into
masterfrom
add_unicorn_eslint_rules

Conversation

@Crabcyborg
Copy link
Copy Markdown
Contributor

@Crabcyborg Crabcyborg commented Jan 6, 2026

This is currently using a fairly outdated version as our version of ES Lint is outdated as well.

Rules applied so far

  • Prefer remove / append over removeChild / appendChild.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jan 6, 2026

Walkthrough

Replaces many DOM insertion/removal calls across the codebase: appendChildappend and removeChildremove (and a few uses of parentNode.removeChildremove()). No functional/control-flow changes besides one or two removed early returns in style-related handlers.

Changes

Cohort / File(s) Summary
Admin Panel UI Components
js/admin/applications.js, js/admin/dom.js, js/admin/embed.js, js/admin/style.js, js/src/admin/admin.js, js/src/admin/styles.js
Replaced appendChild/removeChild with append/remove across modal, tooltip, stylesheet, and general UI assembly paths; removed a couple early returns in style.js event handling.
Form & Addon Management
js/formidable.js, js/src/admin/addon-state.js, js/src/admin/upgrade-popup.js, js/src/form-templates/elements/applicationTemplatesElement.js
Switched insertion/removal APIs for honeypot removal, hidden-input/token insertions, settings/options insertion, and upgrade image appending.
Web Components & Tab UI
js/src/web-components/frm-tab-navigator-component/frm-tab-navigator-component.js, js/src/web-components/frm-web-component.js, js/formidable-web-components.js
Converted component initialization and render flows to use append for stylesheet, view, tabs, delimiters, and tab containers.
Token & Input Components
js/src/settings-components/components/token-input/token-elements.js, square/js/frontend.js
Replaced appendChild with append when inserting token/hidden input nodes.
Plugin, Floating UI & Search
js/packages/floating-links/s11-floating-links.js, js/plugin-search.js, js/src/welcome-tour/ui/spotlight.js, js/welcome-tour.js
Modernized DOM insertion/removal (append/remove) for slide-ins, dismiss-link relocation, spotlight attachment, and welcome-tour wrapper adjustments.
Square Integration & Action
square/js/action.js
Updated dropdown option appends to use append.
Misc / Build Artifacts
js/formidable-settings-components.js, js/formidable_styles.js, js/frm_testing_mode.js
Minor modernizations or formatting-only diffs; appendChild→append where style elements are injected; some files are formatting-parity only.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Suggested reviewers

  • lauramekaj1

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 7.08% 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: adding unicorn ESLint rules to the codebase, which aligns with the PR's objective of updating ESLint rules and the implemented changes.
Description check ✅ Passed The description is related to the changeset, explaining the outdated ESLint version and the specific unicorn rule applied (prefer remove/append over removeChild/appendChild), which matches the actual code changes across multiple files.
✨ 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 e16d903 and a4229fb.

📒 Files selected for processing (11)
  • js/admin/applications.js
  • js/admin/dom.js
  • js/admin/embed.js
  • js/form-templates.js
  • js/formidable-settings-components.js
  • js/formidable-web-components.js
  • js/formidable_admin.js
  • js/formidable_styles.js
  • js/frm_testing_mode.js
  • js/src/admin/admin.js
  • js/welcome-tour.js
🚧 Files skipped from review as they are similar to previous changes (1)
  • js/admin/dom.js
🧰 Additional context used
🪛 Biome (2.1.2)
js/formidable_styles.js

[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: Do not use the t variable name as a label

The variable is declared here

Creating a label with the same name as an in-scope variable leads to confusion.

(lint/suspicious/noLabelVar)


[error] 1-1: Do not reassign a function declaration.

Reassigned here.

Use a local variable instead.

(lint/suspicious/noFunctionAssign)


[error] 1-1: Do not reassign a function declaration.

Reassigned here.

Use a local variable instead.

(lint/suspicious/noFunctionAssign)


[error] 1-1: Do not reassign a function declaration.

Reassigned here.

Use a local variable instead.

(lint/suspicious/noFunctionAssign)

js/welcome-tour.js

[error] 2-2: This case is falling through to the next case.

Add a break or return statement to the end of this case to prevent fallthrough.

(lint/suspicious/noFallthroughSwitchClause)


[error] 2-2: This case is falling through to the next case.

Add a break or return statement to the end of this case to prevent fallthrough.

(lint/suspicious/noFallthroughSwitchClause)


[error] 2-2: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 2-2: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 2-2: Do not use the t variable name as a label

The variable is declared here

Creating a label with the same name as an in-scope variable leads to confusion.

(lint/suspicious/noLabelVar)


[error] 2-2: Do not reassign a function declaration.

Reassigned here.

Use a local variable instead.

(lint/suspicious/noFunctionAssign)


[error] 2-2: Do not reassign a function declaration.

Reassigned here.

Use a local variable instead.

(lint/suspicious/noFunctionAssign)


[error] 2-2: Do not reassign a function declaration.

Reassigned here.

Use a local variable instead.

(lint/suspicious/noFunctionAssign)


[error] 2-2: Do not reassign a function declaration.

Reassigned here.

Use a local variable instead.

(lint/suspicious/noFunctionAssign)


[error] 2-2: Do not reassign a function declaration.

Reassigned here.

Use a local variable instead.

(lint/suspicious/noFunctionAssign)


[error] 2-2: Do not reassign a function declaration.

Reassigned here.

Use a local variable instead.

(lint/suspicious/noFunctionAssign)


[error] 2-2: Do not reassign a function declaration.

Reassigned here.

Use a local variable instead.

(lint/suspicious/noFunctionAssign)


[error] 2-2: Do not reassign a function declaration.

Reassigned here.

Use a local variable instead.

(lint/suspicious/noFunctionAssign)

js/formidable-web-components.js

[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: Do not reassign a function declaration.

Reassigned here.

Use a local variable instead.

(lint/suspicious/noFunctionAssign)


[error] 1-1: Do not reassign a function declaration.

Reassigned here.

Use a local variable instead.

(lint/suspicious/noFunctionAssign)


[error] 1-1: Do not reassign a function declaration.

Reassigned here.

Use a local variable instead.

(lint/suspicious/noFunctionAssign)


[error] 1-1: Do not reassign a function declaration.

Reassigned here.

Use a local variable instead.

(lint/suspicious/noFunctionAssign)


[error] 1-1: Do not reassign a function declaration.

Reassigned here.

Use a local variable instead.

(lint/suspicious/noFunctionAssign)


[error] 1-1: Do not reassign a function declaration.

Reassigned here.

Use a local variable instead.

(lint/suspicious/noFunctionAssign)


[error] 1-1: Do not reassign a function declaration.

Reassigned here.

Use a local variable instead.

(lint/suspicious/noFunctionAssign)


[error] 1-1: Do not reassign a function declaration.

Reassigned here.

Use a local variable instead.

(lint/suspicious/noFunctionAssign)


[error] 1-1: Do not reassign a function declaration.

Reassigned here.

Use a local variable instead.

(lint/suspicious/noFunctionAssign)


[error] 1-1: Do not reassign a function declaration.

Reassigned here.

Use a local variable instead.

(lint/suspicious/noFunctionAssign)

js/formidable-settings-components.js

[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 1-1: Do not reassign a function declaration.

Reassigned here.

Use a local variable instead.

(lint/suspicious/noFunctionAssign)


[error] 1-1: Do not reassign a function declaration.

Reassigned here.

Use a local variable instead.

(lint/suspicious/noFunctionAssign)


[error] 1-1: Do not reassign a function declaration.

Reassigned here.

Use a local variable instead.

(lint/suspicious/noFunctionAssign)


[error] 1-1: Do not reassign a function declaration.

Reassigned here.

Use a local variable instead.

(lint/suspicious/noFunctionAssign)


[error] 1-1: Do not reassign a function declaration.

Reassigned here.

Use a local variable instead.

(lint/suspicious/noFunctionAssign)

js/frm_testing_mode.js

[error] 2-2: This case is falling through to the next case.

Add a break or return statement to the end of this case to prevent fallthrough.

(lint/suspicious/noFallthroughSwitchClause)


[error] 2-2: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)


[error] 2-2: Do not use the e variable name as a label

The variable is declared here

Creating a label with the same name as an in-scope variable leads to confusion.

(lint/suspicious/noLabelVar)


[error] 2-2: Do not reassign a function declaration.

Reassigned here.

Use a local variable instead.

(lint/suspicious/noFunctionAssign)


[error] 2-2: Do not reassign a function declaration.

Reassigned here.

Use a local variable instead.

(lint/suspicious/noFunctionAssign)


[error] 2-2: Do not reassign a function declaration.

Reassigned here.

Use a local variable instead.

(lint/suspicious/noFunctionAssign)


[error] 2-2: Do not reassign a function declaration.

Reassigned here.

Use a local variable instead.

(lint/suspicious/noFunctionAssign)

⏰ 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). (1)
  • GitHub Check: Cypress
🔇 Additional comments (22)
js/admin/embed.js (2)

151-151: LGTM! Modern DOM API usage.

The systematic replacement of appendChild() with append() aligns with modern JavaScript best practices and the ESLint unicorn plugin recommendations. The append() method is more flexible and concise while maintaining identical functionality for single-element insertion.

Also applies to: 155-155, 185-185, 197-197, 284-284, 289-289, 291-292, 316-316, 325-325, 333-333, 351-351, 357-359, 361-361, 410-410, 431-431, 434-435, 496-496


498-498: LGTM! Modern element removal.

Using element.remove() is the preferred modern approach over parent.removeChild(element). This simplifies the code and improves readability.

js/admin/applications.js (1)

71-71: LGTM! Modern DOM API usage.

The replacement of appendChild() with append() throughout this file aligns with the PR objective to modernize DOM manipulation. All changes are functionally equivalent since single nodes are being passed, and the modern append() method improves consistency with contemporary JavaScript practices.

Also applies to: 75-75, 141-142, 147-147, 190-190, 209-209, 250-250, 276-277, 307-307, 309-309, 312-312, 317-317, 366-366, 370-370

js/welcome-tour.js (1)

2-2: Verify the build output and test welcome tour functionality.

The extra IIFE wrapper around the module is likely a result of the build process. While this typically doesn't affect functionality, it's worth verifying that the welcome tour initializes and behaves correctly, especially regarding timing-sensitive interactions.

Note: The static analysis hints flagging issues in this minified code can be safely ignored, as such patterns are expected in bundled/minified JavaScript.

js/src/admin/admin.js (18)

285-291: empty helper: append/remove migration looks correct

Using while ( $obj.firstChild ) { $obj.firstChild.remove(); } preserves the previous behavior of clearing all children while slightly simplifying the code. No issues detected.


1238-1241: Field group/section cleanup: .append/.remove use is safe

Switching to parentNode.append( endDivider ) and child.remove() / closestFieldBox.remove() keeps the same ordering and removal semantics as the old appendChild/removeChild calls. All targets are Elements (from .children/classList), so .remove() is valid and iteration uses snapshots, avoiding mutation issues.

Also applies to: 1275-1292, 1133-1137


1375-1402: Field group controls & dropdown: DOM insertion unchanged semantically

builderPage.append( controls ), controls.append(...), and dropdown.append(...) simply replace earlier appendChild calls. Controls are still created once, attached to the builder root, then moved into rows as before. No behavioral change beyond the modern API.

Also applies to: 1405-1424, 1440-1473


1711-1714: Drag‑and‑drop placeholder handling: .remove is equivalent

Replacing the drag placeholder via placeholder.parentNode.insertBefore( loading, placeholder ); placeholder.remove(); is equivalent to the prior parent‑based removal, and the later use of the loading placeholder is unchanged. No drag/drop edge cases introduced.

Also applies to: 1756-1785


2243-2254: Temporary anchor in showLimitModal: updated remove/append usage is fine

Appending the temporary <a> with wrapper.append(...) and cleaning it up via temporaryAnchor.remove() maintains the same lifecycle and avoids leaking the helper element. Behavior of the limit modal trigger is unchanged.


2698-2716: Field action dropdown items: append on anchors and list items

Using anchor.append(...) and li.append( anchor ); ul.append( li ); is a direct modern replacement for the previous appendChild calls. Order of text/icon and items in the menu remains identical.


2732-2755: wrapFieldLi / wrapFieldLiInPlace: node moves remain correct

Switching to wrapper.append( field ), ul.append( li ) keeps the previous behavior of moving the existing field <li> into a new wrapper and grid <ul>. Since append moves nodes, row restructuring and subsequent sortable/draggable setup remain intact.

Also applies to: 2757-2776


3166-3178: popCalcFields: building calc field list with append is equivalent

Constructing anchors and list items with a.append(...), li.append( a ), and list.append( li ) matches the old structure exactly (icon/text, data attributes, ordering). No changes to event wiring or data attributes.


4147-4150: Field group layout popup: popup assembly unchanged in behavior

All updates here are div()/span() construction changes from appendChild to append. The hierarchy (popup → wrapper → title/labels/rows/buttons) and focus handling are preserved, and layout calculations still use the same DOM structure. No new edge cases identified.

Also applies to: 4174-4207, 4267-4286, 4298-4323, 4349-4357, 4507-4564


4951-5007: Multiselect popup: appending controls with modern APIs

mergeOption.append( caret ), popup.append( mergeOption / separator / deleteOption ), and later appending the popup to post-body-content are all straightforward replacements. Popup behavior (positioning, buttons) remains the same.

Also applies to: 5008-5012


5175-5179: Moving open modals out of field options: form.append(modal) is fine

modal.closest( 'form' ).append( modal ); keeps the previous semantics of re‑parenting the inline modal to the form root. This still avoids the “modal deleted with field” issue the helper is designed for.


5381-5387: Option text focus: appending hidden optionmap input

this.parentNode.append( input ); simply modernizes the hidden input insertion; the mapping in optionMap[fieldId][this.value] still points at the same element. No change in mapping behavior.


6047-6077: resetSingleOpt: child node cleanup via .remove()

Within the text‑node iteration, node.remove() replaces parentNode.removeChild(node) while preserving the logic that keeps only the main label node (or image container). Because we only touch nodes after the first input sibling and operate on an array snapshot, there’s no mutation hazard.


6139-6159: Multiple‑choice options: addRadioCheckboxOpt and select option creation

All container.append(...) and field.append( opt ) calls are 1:1 replacements for appendChild. Option order, checked state, and price/data attributes are unaffected.

Also applies to: 6351-6352


7070-7072: triggerSubmit and post‑save cleanup: .append/.remove usage is sound

Appended hidden submit buttons and the timed removal of .frm_updated_message via this.remove() behave exactly like the prior DOM manipulations, with no event or lifecycle changes.

Also applies to: 7231-7233


7308-7352: Upgrade tab population: appending cloned modal pieces

Using container.append( upgradeActions || upgradeButton ), appendClonedModalElementToContainer(...), and container.append( getScreenshotWrapper(...) ) keeps the same composed UI from the upgrade modal, just using the modern API. Cloning and license‑level adjustments are unchanged.

Also applies to: 7347-7348


7480-7481: Form action duplication/creation & settings reparenting

All append calls here (adding new actions, placeholders, and moving settings blocks via a DocumentFragment) are direct substitutes for appendChild. Reparenting order and the end‑marker logic in moveFieldSettings remain correct.

Also applies to: 7551-7552, 7573-7577, 7803-7807


7950-7953: Other scattered .append/.remove replacements

Across the remaining hunks (logic rows, taxonomy/meta rows, inbox banners, template install, admin footer links, select‑dependency wiring, etc.) the changes are purely mechanical appendChildappend and removeChildremove updates. Target nodes are Elements, parentage is unchanged, and no code depends on the old return values of appendChild. Behavior should be identical while satisfying the new ESLint unicorn rules.

Also applies to: 8035-8052, 8151-8153, 8951-8957, 9024-9030, 9104-9127, 9158-9171, 9239-9245, 9354-9355, 9479-9483, 9513-9515, 9616-9617, 9690-9704, 9759-9775, 9797-9827, 9874-9875, 9937-9941, 10018-10023, 10100-10101, 10219-10221


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.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
js/admin/dom.js (1)

866-866: Consider completing the migration to append() for consistency.

This line still uses appendChild() while the rest of the file has been migrated to append(). For consistency with the PR's systematic modernization effort, consider updating this line as well.

🔎 Proposed change for consistency
-	node.childNodes.forEach( child => newNode.appendChild( cleanNode( child ) ) );
+	node.childNodes.forEach( child => newNode.append( cleanNode( child ) ) );
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 702ecba and e16d903.

⛔ Files ignored due to path filters (3)
  • .eslintrc.json is excluded by !**/*.json
  • package-lock.json is excluded by !**/package-lock.json, !**/*.json
  • package.json is excluded by !**/*.json
📒 Files selected for processing (18)
  • js/admin/applications.js
  • js/admin/dom.js
  • js/admin/embed.js
  • js/admin/style.js
  • js/formidable.js
  • js/packages/floating-links/s11-floating-links.js
  • js/plugin-search.js
  • js/src/admin/addon-state.js
  • js/src/admin/admin.js
  • js/src/admin/styles.js
  • js/src/admin/upgrade-popup.js
  • js/src/form-templates/elements/applicationTemplatesElement.js
  • js/src/settings-components/components/token-input/token-elements.js
  • js/src/web-components/frm-tab-navigator-component/frm-tab-navigator-component.js
  • js/src/web-components/frm-web-component.js
  • js/src/welcome-tour/ui/spotlight.js
  • square/js/action.js
  • square/js/frontend.js
🧰 Additional context used
🧬 Code graph analysis (7)
js/packages/floating-links/s11-floating-links.js (5)
js/admin/applications.js (5)
  • frmDom (9-9)
  • frmDom (10-10)
  • frmDom (11-11)
  • frmDom (12-12)
  • frmDom (13-13)
js/admin/embed.js (2)
  • frmDom (9-9)
  • frmDom (10-10)
js/admin/style.js (5)
  • frmDom (23-23)
  • frmDom (24-24)
  • frmDom (25-25)
  • frmDom (26-26)
  • frmDom (1162-1162)
js/src/admin/addon-state.js (1)
  • frmDom (3-3)
js/src/admin/admin.js (3)
  • frmDom (241-241)
  • frmDom (242-242)
  • frmDom (243-243)
square/js/frontend.js (2)
stripe/js/frmstrp.js (1)
  • thisForm (2-10)
js/admin/legacy-views.js (1)
  • thisForm (5-5)
js/admin/style.js (2)
js/admin/applications.js (1)
  • card (265-271)
js/src/admin/admin.js (1)
  • card (9511-9511)
js/src/admin/upgrade-popup.js (1)
js/src/admin/addon-state.js (1)
  • upgradeMessage (227-227)
js/src/form-templates/elements/applicationTemplatesElement.js (2)
js/admin/applications.js (1)
  • elements (25-30)
js/admin/dom.js (1)
  • elements (233-233)
js/src/admin/addon-state.js (1)
js/src/admin/upgrade-popup.js (1)
  • addonStatus (34-34)
js/src/admin/admin.js (1)
js/admin/dom.js (7)
  • a (27-33)
  • option (180-180)
  • modal (12-109)
  • modal (14-14)
  • modal (607-607)
  • element (239-239)
  • element (593-593)
⏰ 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). (9)
  • GitHub Check: PHP 8 tests in WP 6.9
  • GitHub Check: PHP 7.4 tests in WP 6.9
  • GitHub Check: Cypress
  • GitHub Check: PHP 7.4 tests in WP 6.9
  • GitHub Check: PHP 8 tests in WP 6.9
  • GitHub Check: Cypress
  • GitHub Check: Cypress
  • GitHub Check: PHP 7.4 tests in WP 6.9
  • GitHub Check: PHP 8 tests in WP 6.9
🔇 Additional comments (27)
square/js/action.js (2)

120-120: LGTM! Clean modernization.

The change from appendChild to append is correct and maintains identical functionality for single-node insertion.


171-171: LGTM! Consistent with the modernization effort.

The append() method correctly replaces appendChild() with no functional change.

js/src/settings-components/components/token-input/token-elements.js (1)

73-73: LGTM! Correct modernization.

The replacement of appendChild with append is functionally equivalent and improves code consistency.

square/js/frontend.js (1)

124-125: LGTM! Modernized DOM manipulation.

Both append() calls correctly replace the previous appendChild() pattern with no change in functionality.

js/plugin-search.js (1)

38-39: LGTM! Clean refactor for element relocation.

The modernized approach correctly removes the parent node and re-attaches dismissLink to a new container. The dismissLink reference remains valid throughout the operation.

js/src/form-templates/elements/applicationTemplatesElement.js (1)

114-114: LGTM! Consistent modernization.

The change to append() is correct and aligns with the project-wide API modernization.

js/src/welcome-tour/ui/spotlight.js (1)

27-27: LGTM! Modern DOM API usage.

The modernization from appendChild() to append() is a clean improvement that aligns with current JavaScript standards.

js/formidable.js (2)

1359-1359: LGTM! Cleaner removal syntax.

The modernization from parentNode.removeChild() to remove() is more concise and direct.


1885-1903: LGTM! Modern DOM API usage for form submission.

The updates to use append() for adding the antispam token and unique ID inputs are clean modernizations that maintain identical functionality.

js/packages/floating-links/s11-floating-links.js (1)

78-78: LGTM! Consistent modernization across the component.

All appendChild() to append() conversions throughout the floating links component construction are clean and maintain the existing DOM hierarchy and behavior.

Also applies to: 119-119, 157-157, 192-196, 236-236

js/src/web-components/frm-web-component.js (1)

40-41: LGTM! Modern API in base component class.

The conversion to append() for adding styles and views in the web component render method is a clean modernization that benefits all derived components.

js/src/admin/styles.js (3)

66-66: LGTM! Modern DOM API for style injection.

The modernization to append() for adding the inline style element to the document head is clean and maintains identical behavior.


136-136: LGTM! Modern API for hover element insertion.

The conversion to append() for adding the hover element maintains identical functionality while using the modern standard.


221-221: LGTM! Modern API for temporary element insertion.

The modernization to append() for the clipboard fallback's temporary input element is correct and maintains identical behavior.

js/admin/applications.js (1)

71-71: LGTM: Clean modernization to append().

All appendChild()append() replacements are safe. The return values weren't being used, and all arguments are DOM nodes (not strings), so behavior is preserved.

Also applies to: 75-75, 141-142, 190-190, 209-209, 250-250, 276-277, 307-307, 309-309, 312-312, 317-317, 366-366, 370-370

js/src/admin/upgrade-popup.js (1)

90-95: LGTM: Safe modernization.

The append() replacement is correct. The upsell image insertion works identically with the new API.

js/src/admin/addon-state.js (1)

141-141: LGTM: Safe modernization.

The append() replacement preserves functionality. The return value wasn't being used.

js/admin/style.js (2)

589-589: LGTM: Safe modernization.

The append() call correctly adds the hamburger menu to the card.


1127-1133: LGTM: Clean CSS reload implementation.

Both changes are safe:

  • style.remove() cleanly replaces parentNode.removeChild(style)
  • head.append(newStyle) replaces appendChild(newStyle)

The load event listener ensures the old stylesheet is only removed after the new one loads, preventing a flash of unstyled content.

js/admin/embed.js (1)

151-151: LGTM: Comprehensive modernization.

All appendChild()append() replacements are safe and correct:

  • No return values were being used
  • All arguments are DOM nodes (not strings)
  • Insertion order and functionality preserved throughout modal construction, page dropdown, form creation, and embed examples

Also applies to: 155-155, 185-185, 197-197, 284-284, 289-289, 291-292, 351-351, 357-359, 361-361, 410-410, 431-431, 434-435, 496-496

js/admin/dom.js (4)

36-53: LGTM! Modal creation modernized correctly.

The migration to append() for constructing modal sections (top, content, footer) is clean and maintains the same functionality.


599-599: LGTM! Helper functions updated consistently.

The modal helper and creation functions now use the modern append() API correctly.

Also applies to: 610-611


746-758: LGTM! Tag builder function updated correctly.

The tag() function now uses append() for adding children. The explicit text node creation at line 749 is good practice for clarity, even though append() can accept strings directly.


777-777: LGTM! Remaining DOM helpers updated consistently.

The svg(), success(), and redraw() functions all correctly use the modern append() API.

Also applies to: 799-799, 815-815

js/src/web-components/frm-tab-navigator-component/frm-tab-navigator-component.js (1)

24-26: Code change is correct; no IE11 compatibility concern for this project.

The migration from appendChild() to append() is appropriate. The project uses web components (which themselves lack IE11 support without polyfills) and shows no IE11-specific configuration or polyfill setup, indicating IE11 is not a targeted browser. The modern append() API is the correct choice for this codebase.

js/src/admin/admin.js (2)

285-291: Node.remove() migrations look correct; just confirm browser support assumptions

Across these blocks you’ve switched from parent.removeChild(child)/jQuery(...).remove() to child.remove() (or equivalent):

  • empty helper (Line 285–291)
  • maybeDeleteAnEmptyFieldGroup (Line 1133–1138)
  • maybeDeleteEmptyFieldGroups (Line 1245–1247)
  • deleteEmptyDividerWrappers (Line 1282–1290)
  • Field‑group delete decoy (Line 4047–4051)
  • moveOpenModalsOutOfFieldOptions (Line 5171–5179)
  • resetSingleOpt cleanup of stray text nodes (Line 6059–6078)
  • triggerSubmit temporary button cleanup (Line 7066–7071)
  • afterFormSave transient message removal (Line 7229–7233)
  • trashTemplate card removal (Line 9510–9515)

In all cases:

  • The target is a concrete DOM node (not a jQuery wrapper), so .remove() is valid.
  • No call site was using the return value of removeChild, so behavior and data flow are preserved.
  • Loop patterns that remove nodes (like deleteEmptyDividerWrappers) already worked with removal; switching to .remove() doesn’t introduce new mutation issues.

Only thing to double‑check is that your supported browser matrix (or any polyfills you ship) explicitly allows reliance on Node.prototype.remove (IE11 would need a polyfill, but this code already used other modern constructs like optional chaining).

Also applies to: 1133-1138, 1245-1247, 1282-1290, 4047-4051, 5171-5179, 6059-6078, 7066-7071, 7229-7233, 9510-9515


1393-1401: appendChild→append transitions are behavior‑preserving and consistent with Unicorn rules

These hunks cover all the appendChildappend moves (and related new append sites):

  • Field group controls setup (updateFieldGroupControls, setFieldControlsHtml, getFieldControlsDropdown) (Line 1393–1401, 1420–1424, 1465–1472)
  • Calc‑field shortcode list (popCalcFields) (Line 1666–1680)
  • Limit modal temp anchor (showLimitModal) (Line 2248–2254)
  • Wrapping / re‑wrapping fields (wrapFieldLi, wrapFieldLiInPlace) (Line 2735–2755, 2772–2777)
  • Field‑group duplication helper list item injection (Line 4114–4123)
  • Field‑group layout popup construction (getFieldGroupPopup, row‑layout helpers) (Line 4185–4207, 4272–4293, 4320–4323, 4349–4360, 4512–4564)
  • Field‑group message insertion (maybeShowFieldGroupMessage) (Line 4842–4856)
  • Multiselect popup construction (getFieldMultiselectPopup) (Line 4972–5009)
  • Option‑mapping hidden input insertion (onOptionTextFocus) (Line 5375–5387)
  • Conditional‑logic option rebuilding (adjustConditionalLogicOptionOrders) (Line 5536–5540)
  • <select> option population (fillDropdownOpts, logic value select) (Line 6335–6351, 6631–6639)
  • Action duplication and creation (copyFormAction, addFormAction) (Line 7472–7480, 7539–7555)
  • moveFieldSettings DocumentFragment assembly (Line 7776–7813)
  • Admin footer links relocation (addAdminFooterLinks) (Line 10210–10219)

Checks:

  • None of these sites rely on the return value of appendChild, so switching to append doesn’t break any chaining or assignments.
  • Where existing nodes are re‑parented (e.g., moving controls, modals, or list items), append still moves the node (same as appendChild), so ordering and DOM structure stay intact.
  • append is only called on real DOM nodes / DocumentFragments (not jQuery objects), with single Node arguments, which matches the previous usage.

As with .remove(), this does assume your runtime targets browsers that support Node.prototype.append (or that you polyfill it). Given the rest of the file already uses modern DOM APIs, this modernization is consistent; just confirm that build + browser support expectations are aligned.

Also applies to: 1420-1424, 1465-1472, 1666-1680, 2248-2254, 2735-2755, 2772-2777, 4114-4123, 4185-4207, 4272-4293, 4320-4323, 4349-4360, 4512-4564, 4842-4856, 4972-5009, 5375-5387, 5536-5540, 6335-6351, 6631-6639, 7472-7480, 7539-7555, 7776-7813, 10210-10219

@Crabcyborg Crabcyborg merged commit 089c91e into master Jan 6, 2026
15 checks passed
@Crabcyborg Crabcyborg deleted the add_unicorn_eslint_rules branch January 6, 2026 20:29
stephywells pushed a commit that referenced this pull request Apr 4, 2026
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