UDOIT icon indicating copy to clipboard operation
UDOIT copied to clipboard

Clean up JS

Open rob-3 opened this issue 3 years ago • 1 comments

We should take a look through the JavaScript side of the project and clean up things like unused variables and fix any unjustified warnings.

We could also consider updating to function components, but that may be more of a long-term goal. The class components we have are still 100% supported by React.

rob-3 avatar Feb 23 '22 15:02 rob-3

We may want to consider splitting these into separate issues, as the list is pretty long.

This involves cleaning up some of the code in assets/js/. The main items are:

  • [x] Rewriting the classes in assets/js/Services into plain functions. There is no good reason for these classes with no fields or state to exist in JavaScript, which has free functions. This is basically turning things like:
export default class Ufixit {
  returnIssueForm(activeIssue) {
    if (activeIssue) {
      if (UfixitForms.hasOwnProperty(activeIssue.scanRuleId)) {
        if (!activeIssue.sourceHtml.includes("data-type-module")) {
          return UfixitForms[activeIssue.scanRuleId]
        }
      }
    }

    return UfixitReviewOnly
  }
}

into

export function returnIssueForm(activeIssue) {
  if (activeIssue) {
    if (UfixitForms.hasOwnProperty(activeIssue.scanRuleId)) {
      if (!activeIssue.sourceHtml.includes("data-type-module")) {
        return UfixitForms[activeIssue.scanRuleId]
      }
    }
  }

  return UfixitReviewOnly
}

and updating the usages and imports elsewhere. You will also get to remove a bunch of instances of this after this is done.

  • [x] Removing any unused imports
  • [ ] Removing any unused variables/parameters
  • [x] Remove use of eval()! Using eval() is a bad idea is virtually all circumstances
  • [ ] Modernize JS style (remove var, use arrow functions where appropriate, etc)
  • [ ] Consider using public fields syntax for class components to avoid the need for .bind() everywhere
  • [ ] Update JS dependencies. Several packages in package.json are not the latest version or even the latest minor version. yarn upgrade can do some changes, but things like the @instructure/ui ilbraries need to be manually bumped and tested and any breaking changes fixed.
  • [ ] (Possibly) Updating to function components. Someone with working knowledge of class-based and function-based React components will need to do this
  • [ ] (Possibly) Implement some kind of JavaScript style guide for the repo and consider using a code formatter like Prettier to enforce that style. Currently, files have things like mixed 2 and 4 spaces and semicolons strewn at random.

rob-3 avatar Apr 19 '22 18:04 rob-3