Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add One Transaction Report View #36934
Add One Transaction Report View #36934
Changes from 13 commits
53ada16
df1b56c
b841b6a
7ec2e5c
353b740
42adcea
66f498b
de66987
74a42f6
1bf4c04
6153bab
d436ffe
b722725
b13fc4b
452d012
cac6d81
6a08ba5
1674735
f3f5e52
90bc361
0f21504
4bb0313
35d0264
c941d23
2a02be8
b5ae144
f6cfcb3
0140912
db05afc
c274090
618b5ac
31cf018
6b2d28a
1b27a33
d36bc98
686a2dc
7994917
1003a1f
de600c8
034b93f
716023c
3b88172
45c7fc1
f9f3607
e3be337
1090c5b
a4926d3
7dd2384
fec3aa3
5438f69
1aac6e0
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
These methods are anti-patterns, like the ones being removed in #27262.
The problem with them is in how they are used. If a view component uses these methods, then the view is accessing data that is not connected via
withOnyx()
. Meaning that if the onyx data changes, the view won't re-render and it will have stale data.What you'll have to do instead is make sure the component is connected to the
REPORT
collection, and then using they key function and maybe a selector, you'll have to look up all this stuff from the connected data.I hope that makes sense.
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.
I was not awaer of that tracking/ clean up issue, thanks for raising this. It makes sense, annoyingly this is easy to miss, these methods are convenient so that you might not realize the consequence
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.
Why is this method being exported? It doesn't look like it is used anywhere outside of this file (and this is one of those anti-patterns we discussed). In fact, would you mind adding this method to https://github.com/Expensify/App/blob/main/tests/actions/EnforceActionExportRestrictions.ts so that it doesn't get exported and used somewhere else?
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.
In fact, now I'm also worried that any of the methods in this file that use
isOneTransactionReport()
are being exported and used improperly.