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

Replace many lodash _.get() calls with native ES expressions #2515

Merged
merged 2 commits into from
Jan 8, 2025

Conversation

jake-low
Copy link
Contributor

@jake-low jake-low commented Jan 1, 2025

This PR removes most (about 97%) of calls to lodash's _get() method, which retrieves properties from nested objects, and returns undefined when the path does not exist:

let o = { foo: { x: 2 }, bar: false };
_get(o, 'foo.x'); // returns 2
_get(o, 'bar.baz'); // returns undefined

ECMAScript 2020 added native support for this with the optional chaining (?.) and nullish-coalescing (??) operators. I think it's a good idea to switch to this pattern for a couple of reasons:

  1. reducing the bundle size by removing dependencies on lodash modules
  2. the semantics of native methods are familiar to more developers, making code easier to understand and contribute to
  3. lodash's _get() method is opaque to typecheckers, making it harder to adopt type checking in MapRoulette

For those reasons, I took a stab at cleaning up usage of _get() in the codebase. Most of the changes in this PR were done by a script which I adapted to this purpose.

There are a few places where the second argument to _get() comes from a function argument or subroutine return value, rather than being a string literal. It's not possible to tell automatically in these cases if the value is a simple identifier (safe to transform) or a complex keypath (requiring refactoring by hand). To avoid accidentally breaking any code, I left those cases alone for now.

Once this is merged there are some other lodash methods I'd also like to replace with native equivalents (_keys() with Object.keys(), _map with Array.prototype.map(), etc). Those should be easier than _get to refactor automatically (find and replace will suffice in most cases).

CollinBeczak
CollinBeczak previously approved these changes Jan 8, 2025
@CollinBeczak
Copy link
Collaborator

LGTM, i see no change in all of the normal workflows, and the changelog looks good. Just needs to be rebased and should be good to merge.

@jake-low jake-low merged commit bbceb4d into main Jan 8, 2025
5 of 6 checks passed
@jake-low jake-low deleted the jlow/less-lodash branch January 8, 2025 19:45
@CollinBeczak CollinBeczak mentioned this pull request Feb 4, 2025
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