Skip to content

refactor: modernize JS to pass ESLint - #5

Merged
jjroelofs merged 1 commit into
1.xfrom
refactor/eslint-fixes
Aug 6, 2026
Merged

jjroelofs merged 1 commit into
1.xfrom
refactor/eslint-fixes

Conversation

@jjroelofs

Copy link
Copy Markdown
Contributor

Summary

  • Modernise js/rl-sorting-tracking.js to pass ESLint with @antfu/eslint-config (zero errors)
  • Convert all var declarations to const/let
  • Use arrow functions, object shorthand methods, template literals, and trailing commas
  • Destructure viewSettings to pull only the three properties actually used (removes the unused entityIds assignment)
  • Hoist storageKey out of the links.forEach loop since it depends only on experimentId, not the individual link
  • Convert sendBatchedTurns from a hoisted function declaration to a const arrow for consistency with the rest of the file
  • Remove trailing whitespace; add missing trailing newline
  • No functional changes

Closes #4

Test plan

  • Run npx eslint . and confirm zero errors
  • Verify RL Sorting impression tracking still fires (IntersectionObserver batched turns via sendBeacon)
  • Verify click reward tracking still fires (sessionStorage dedup, sendBeacon to RL endpoint)
  • Confirm no regressions in Views pages with RL Sorting enabled

@jjroelofs jjroelofs left a comment

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.

Reviewed the full diff against the old file; no behavioral changes found. Looks good.

What I verified:

  • The destructuring of settings.rlSorting.views[viewDisplayKey] correctly drops only entityIds, which was assigned but never read in the old code. The three properties that are used (experimentId, entityUrlMap, rlEndpointUrl) all survive, and the !experimentId || !rlEndpointUrl guard still runs after it.
  • Hoisting storageKey out of the links.forEach loop is correct: it depends only on experimentId, which is fixed per view, so computing it once is equivalent and the click handlers close over the same value.
  • Converting sendBatchedTurns from a hoisted function declaration to a const arrow is safe here because its only call sites (the setTimeout inside the IntersectionObserver callback) execute after the declaration; there is no temporal dead zone hazard.
  • visibleArms and batchTimer correctly stay let since both are reassigned.

On CI: eslint, drupal-check, changes and emdash-lint are green. The drupal-lint failure is unrelated to this PR; the job dies during setup because composer cannot install phpcompatibility/php-compatibility (it reverts composer.json and exits before any linting runs). That is a CI infrastructure issue that will hit every PR in this repo until the lint action's dependency resolution is fixed.

@jjroelofs
jjroelofs merged commit 39dc3dd into 1.x Aug 6, 2026
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor JS to pass ESLint and modernize codebase

1 participant