Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the typings #1

Conversation

ocket8888
Copy link

@ocket8888 ocket8888 commented Mar 31, 2022

A summary of what I did (not everything was necessary):

  • Angular properties not imported
  • Dynamically adding properties that don't need to be accessed in the template
  • Namespace types not imported
  • Namespace not necessary
  • scope.selected set dynamically but value never read
  • implicit any-typed parameter in click handler
  • click handler JSDoc not bound to any visible symbol - removed
  • parameter to collapseRecurse typed as boolean | null instead of optional
  • unused parameter to closeSelect
  • fuzzy match loops over own properties of string primitive (which typescript doesn't like but appears to work fine in reality) switched to collection loop using of
  • evt parameter of collapse has implicit any type
  • incomplete description on checkFilters
  • simplify scope.$watch callback
  • changed a bunch of const funcName = function() {... to function funcName() {... or fat-arrow functions

oh and one last thing you may not have noticed not a big deal but

TABS > SPACES I CHANGED YOUR FAKE INDENTATION TO REAL INDENTATION

USA USA USA USA

scope used to have an implicit any, which is why you could access things willy-nilly. Everything is now strongly typed.

I couldn't get importing the namespace to work, so I quickly gave up. It's already module-scoped anyway; since you have to access it as an import the namespace doesn't do anything but get in the way imo.

@shamrickus shamrickus merged commit 6ac7819 into shamrickus:tp/tenant-select Apr 1, 2022
@ocket8888 ocket8888 deleted the shamrickus/tp-tenant-selection-typing-fixes branch April 1, 2022 17:13
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.

2 participants