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

Remove as assertions, or fix to mitigate the risk of silent failure #158

Merged
merged 8 commits into from
Oct 17, 2023

Commits on Oct 13, 2023

  1. Replace as T[] assertions with ?? []

    - `as` assertions intended to exclude an empty/nullable type can be replaced by a nullish coalescing operator converting nullable values into an acceptable empty value that doesn't pollute the variable's type signature.
    - TODO: document and expound in MetaMask/contributor-docs#47
    MajorLift committed Oct 13, 2023
    Configuration menu
    Copy the full SHA
    53cf673 View commit details
    Browse the repository at this point in the history
  2. Replace as assertions with redundant type guards

    Sometimes TypeScript's type narrowing and inference doesn't fully work quite like it should. Even in these cases, using `as` assertions should be avoided.
    
    It may seem like we know for certain what the type should be and there's no risk of it breaking, but there's always the possibility that some combination of changes elsewhere might affect the typing, in which case `as` will suppress the alerts TypeScript would otherwise provide us with. This may lead to code breaking silently.
    
    In this case, adding type guards and null checks -- even if they're redundant -- is preferable to the above scenario.
    
    - TODO: document and expound in MetaMask/contributor-docs#47
    MajorLift committed Oct 13, 2023
    Configuration menu
    Copy the full SHA
    f950f3e View commit details
    Browse the repository at this point in the history
  3. Make as assertion rely on inference instead of hard-coded type casting

    If anything changes in the typing of `ChangeCategory` or `unreleasedChanges` that make them inconsistent, this code will fail silently. Relying on type inference instead of explicit type casting will make these line a little less brittle.
    
    - TODO: document and expound in MetaMask/contributor-docs#47
    MajorLift committed Oct 13, 2023
    Configuration menu
    Copy the full SHA
    8da9f21 View commit details
    Browse the repository at this point in the history

Commits on Oct 16, 2023

  1. Fix type narrowing for currentVersion by consolidating conditional …

    …branches
    
    - more consistent logic for `isReleaseCandidate` null check
    - wasted processing overhead due to errors throwing later than strictly necessary
    MajorLift committed Oct 16, 2023
    Configuration menu
    Copy the full SHA
    033cd4e View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    44cc00d View commit details
    Browse the repository at this point in the history
  3. Replace Object.keys with getKnownPropertyNames method

    - removes need for type casting while iterating over object keys
    MajorLift committed Oct 16, 2023
    Configuration menu
    Copy the full SHA
    d788964 View commit details
    Browse the repository at this point in the history

Commits on Oct 17, 2023

  1. Configuration menu
    Copy the full SHA
    a26b0bb View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    7b7eef8 View commit details
    Browse the repository at this point in the history