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 return type of App.allUsers #5708

Merged
merged 2 commits into from
Apr 11, 2023
Merged

Fix return type of App.allUsers #5708

merged 2 commits into from
Apr 11, 2023

Conversation

elle-j
Copy link
Contributor

@elle-j elle-j commented Apr 5, 2023

What, How & Why?

App.allUsers was returning an array of users but now returns a record with the User.id as the key and the User as the value as the intended type.

This closes #5671.

☑️ ToDos

  • 📝 Changelog entry
  • 🚦 Updated tests

Comment on lines 220 to 227
public get allUsers(): Readonly<Record<string, User<FunctionsFactoryType, CustomDataType>>> {
const result: Record<string, User<FunctionsFactoryType, CustomDataType>> = {};
for (const user of this.internal.allUsers) {
result[user.identity] = User.get(user);
}

return result;
}
Copy link
Contributor Author

@elle-j elle-j Apr 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: realm-web uses Object.fromEntries(). Feel free to comment on what is preferred here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lol - I just saw this now 🤦 If I have to choose, I probably like fromEntries because it's shorter (fewer lines and doesn't need the type declared on result) and I generally like a pure function over iterating and mutating an object (the result in this case).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha, despite the extra iteration from needing to create the array (.map) passed to fromEntries?

Copy link
Contributor

@takameyer takameyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :shipit:

@elle-j elle-j merged commit 8c88f3c into main Apr 11, 2023
@elle-j elle-j deleted the lj/app-all-users branch April 11, 2023 09:49
papafe added a commit that referenced this pull request Apr 13, 2023
* main: (41 commits)
  allow useQuery to filter or sort a collection by using a callback (#5513)
  Fix `realm.create` types and 'requiredProperties' on Realm.Object constructor (#5697)
  Fix return type of `App.allUsers` (#5708)
  Removed renamed file
  Remove console.log
  Fix README instructions and bump version
  Bump template versions
  Update Templates (#5702)
  Moved "submodules" wireit task to "postinstall"
  Set up typedoc for realm-react, realm-web (#5709)
  Prepare for vNext (#5711)
  Prepare for 12.0.0-alpha.2 (#5707)
  Build iOS prebuilds in release by default (#5710)
  Use the event.sender as assignee when preparing release
  Updated prepare release workflow to print PR url in summary
  Fixing lint error
  fix the template app links (#5701)
  Expose `Sync` as named export (#5705)
  Enable all tests after bad rebase (#5595)
  Clear test state flag (#5689)
  ...

# Conflicts:
#	packages/realm/src/Realm.ts
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Return a record from App.allUsers instead of an array (v12)
3 participants