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

refactor: prefer native methods to lodash where possible #328

Merged
merged 1 commit into from
May 16, 2022

Conversation

agilgur5
Copy link
Collaborator

Summary

Replace most (not all) lodash methods with native methods

Details

  • _.endsWith -> String.endsWith

  • _.concat -> Array.concat

  • _.each -> Array.forEach

  • _.filter -> Array.filter

  • _.map -> Array.map

  • _.some -> Array.some

  • _.has -> key in Object

  • _.defaults -> Object.assign

  • _.get -> ?. and ?? (optional chaining and nullish coalescing)

  • refactor: replace fairly complicated expandIncludeWithDirs func to just use a few simple forEachs

    • not as FP anymore, more imperative, but much simpler to read IMO
  • refactor: add a getDiagnostics helper to DRY up some code

    • also aids readability IMO
  • a few places are still using lodash, but this paves the way toward removing it or replacing it with much smaller individual deps

    • _.compact still used because Array.filter heavily complicates the type-checking currently
    • _.isFunction still used because while it's a one-liner natively, need to import a function in several places
    • _.merge is a deep merge, so there's no native version of this
  • see also https://github.com/you-dont-need/You-Dont-Need-Lodash-Underscore

Review Notes

Unit tests still pass, so other than looking equivalent and typing equivalently, usage still works equivalently (caveat though: tscache and index aren't yet tested)

@agilgur5 agilgur5 added scope: dependencies Issues or PRs about updating a dependency kind: internal Changes only affect the internals, and _not_ the public API or external-facing docs labels May 14, 2022
- _.endsWith -> String.endsWith

- _.concat -> Array.concat
- _.each -> Array.forEach
- _.filter -> Array.filter
- _.map -> Array.map
- _.some -> Array.some

- _.has -> `key in Object`
- _.defaults -> Object.assign
- _.get -> `?.` and `??` (optional chaining and nullish coalescing)

- refactor: replace fairly complicated `expandIncludeWithDirs` func to
  just use a few simple `forEach`s
  - not as FP anymore, more imperative, but much simpler to read IMO
- refactor: add a `getDiagnostics` helper to DRY up some code
  - also aids readability IMO

- a few places are still using lodash, but this paves the way toward
  removing it or replacing it with much smaller individual deps
  - _.compact still used because Array.filter heavily complicates the
    type-checking currently
  - _.isFunction still used because while it's a one-liner natively,
    need to import a function in several places
    - also the package `lodash.isFunction` is lodash v3 and quite
      different from the v4 implementation, so couldn't replace with it
      unfortunately
  - _.merge is a deep merge, so there's no native version of this
    - but we may remove deep merges entirely in the future (as tsconfig
      doesn't quite perform a deep merge), or could replace this with a
      smaller `lodash.merge` package or similar

- see also https://github.com/you-dont-need/You-Dont-Need-Lodash-Underscore
@ezolenko ezolenko merged commit 197061b into ezolenko:master May 16, 2022
@agilgur5 agilgur5 deleted the refactor-lodash-to-native branch July 2, 2023 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: internal Changes only affect the internals, and _not_ the public API or external-facing docs scope: dependencies Issues or PRs about updating a dependency
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants