Skip to content

Explicitly declare all javascript globals and all eslint rule violations

username-removed-636429 requested to merge clean-no-undef into master

This merge request takes every single external global variable referenced within a javascript file and explicitly marks it with a /* global Foo */ comment block at the top of the script.

This also expands all blanket instances of /* eslint-disable */ with an explicit list of disabled rules. This is useful because if we need to search for violations of a particular rule we can simply grep the codebase for something like no-unused-vars or semi and find all of the places where this rule has yet to be fixed.

Lastly, it also removes and resolves any existing no-undef eslint violations. This is useful for catching mistakes like forgetting to declare a variable with var/let/const which can cause hard to find bugs.

What does this MR do?

  1. Looks for generic uses of /* eslint-disable */ and refactors them into individual rule exceptions.
  2. Looks for all occurrences of /* eslint-disable ... no-undef */ and resolves them by either fixing bugs or declaring globals with /* global Foo */.

Are there points in the code the reviewer needs to double check?

This touches a lot of files, most changes touch nothing other than comment blocks or whitespace. The exceptions are the following 14 files which required some small bug fixes after removing no-undef:

  • api.js
  • breakpoints.js
  • build.js
  • commits.js
  • diff_notes/components/jump_to_discussion.js.es6
  • gfm_auto_complete.js.es6
  • gl_dropdown.js
  • groups_select.js
  • importer_status.js
  • namespace_select.js
  • notes.js
  • preview_markdown.js
  • projects_list.js
  • single_file_diff.js

Why was this MR needed?

Removal of technical debt and some necessary changes to help !7288 (merged)

Screenshots (if relevant)

N/A

Does this MR meet the acceptance criteria?

What are the relevant issue numbers?

N/A

Merge request reports