Skip to content

Conversation

@Szymon20000
Copy link
Contributor

@Szymon20000 Szymon20000 commented Sep 23, 2023

Map is faster than object when we have a lot of items. Using map in this place seems to improve chat switching performance.

Details

Related Issues

GH_LINK

Automated Tests

Linked PRs

* @type {Record<string, Promise>}
*/
this.pendingPromises = {};
this.pendingPromises = new Map();
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you update the docs above and leave a comment why we chose to use Map?

Copy link
Contributor

Choose a reason for hiding this comment

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

I can take care of that

Copy link
Contributor

Choose a reason for hiding this comment

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

What do you mean with the docs though? Docs are only generated from onyx.js, and not from other internal implementation files such as this one

Copy link
Contributor

Choose a reason for hiding this comment

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

@type {Record<string, Promise>} I meant this

@Szymon20000 Szymon20000 marked this pull request as ready for review September 27, 2023 11:40
@Szymon20000 Szymon20000 requested a review from a team as a code owner September 27, 2023 11:40
@melvin-bot melvin-bot bot requested review from youssef-lr and removed request for a team September 27, 2023 11:41
Copy link
Contributor

@mountiny mountiny left a comment

Choose a reason for hiding this comment

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

Thanks!

@mountiny mountiny merged commit eda21d0 into Expensify:main Sep 28, 2023
@ospfranco ospfranco mentioned this pull request Sep 28, 2023
59 tasks
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.

3 participants