-
-
Notifications
You must be signed in to change notification settings - Fork 44
Remove lodash.get() calls #259
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
Conversation
9e31701 to
40c2b5f
Compare
Open Scrobbler
|
||||||||||||||||||||||||||||
| Project |
Open Scrobbler
|
| Branch Review |
codex/remove-lodash-get-usage
|
| Run status |
|
| Run duration | 02m 41s |
| Commit |
|
| Committer | Enrico Lamperti |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
|
0
|
|
|
0
|
|
|
3
|
|
|
0
|
|
|
74
|
| View all changes introduced in this branch ↗︎ | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR removes all instances of lodash.get (and related calls) across the project in favor of optional chaining and nullish coalescing, while also updating tests for new number-based totalPages.
- Replaces lodash.get/hasIn calls with native optional chaining in several transformer files.
- Updates tests and type conversions (e.g., totalPages conversion in userRecentTracks transformer).
- Cleans up similar patterns in API calls, reducer logic, and component error handling.
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/clients/lastfm/transformers/userRecentTracks.transformer.ts | Removed lodash.get calls and replaced them with optional chaining and explicit type conversion for totalPages. |
| src/utils/clients/lastfm/transformers/userRecentTracks.transformer.test.ts | Adjusted test expectations to reflect numeric totalPages. |
| src/utils/clients/lastfm/transformers/userProfile.transformer.ts | Updated the avatar transformation to only process arrays. |
| src/utils/clients/lastfm/transformers/tracksResponse.transformer.ts | Replaced lodash.get with optional chaining for rawTrackList and album assignment. |
| src/utils/clients/lastfm/transformers/topAlbumsResponse.transformer.ts; albumSearchResponse.transformer.ts; albumGetInfoResponse.transformer.ts; trackGetInfo.ts | Similar changes applied for optional chaining in data access. |
| src/utils/clients/discogs/transformers/*.ts | Removed lodash.get calls; applied optional chaining updates in tracks and topAlbums transformers. |
| src/utils/clients/api/transformers/settings.transformer.ts | Replaced lodash.get calls with optional chaining and updated function signature. |
| src/utils/axios.ts | Replaced lodash.get with optional chaining for error handling improvements. |
| src/store/reducers/scrobbleReducer.ts | Updated error message extraction via optional chaining. |
| src/domains/scrobbleUser/ScrobbleUserResults.tsx; SongForm.tsx; AlbumResults.tsx; ScrobbleItem.tsx | Removed lodash.get in favor of native optional chaining for improved readability and type safety. |
Comments suppressed due to low confidence (2)
src/utils/clients/api/transformers/settings.transformer.ts:3
- Consider using a more specific type such as Partial instead of any for the settings parameter to improve type safety.
export function settingsTransformer(settings: any = {}): Settings {
src/utils/clients/discogs/transformers/tracksResponse.transformer.ts:44
- [nitpick] Review the fallback logic and confirm that defaulting to an empty string when no artist name is available is the intended behavior.
artist: sanitizeArtistName((track.artists?.[0]?.name ?? options?.artist) || ''),
Summary
getcalls across the projecttotalPagesTesting
yarn test:unithttps://chatgpt.com/codex/tasks/task_e_6840634bae6c83289d49d6c4be8f057b