Skip to content

[Onyx Audit] Remove try/catch block from getCollectionKey #81830

@JKobrynski

Description

@JKobrynski

Coming from Expensify/react-native-onyx#725 (comment)

E/App Onyx version bump PR #81845

Rory suggested that we should remove the try/catch block from getCollectionKey and return null when no key is found instead. The issues with current solution are:

  • Hidden contract: It's not obvious from reading this code that getCollectionKey throws for non-collection keys. You have to go read its implementation to understand the flow.
  • Expensive error path: If many keys aren't collection members, you're creating exception objects frequently (though JS/TS exceptions are cheaper than some languages)
  • Empty catch: The silent catch with just a comment is a code smell—what if getCollectionKey throws for a different reason? You'd swallow that error too.

We should update the getCollectionKey function and every function that uses it.

Issue OwnerCurrent Issue Owner: @JKobrynski

Metadata

Metadata

Labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions