-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Refactor FlagCommentPage to access parent report directly from Onyx #38356
Changes from 6 commits
2bfd0b5
94e658f
f5f252c
26766e7
4f24c58
444e68d
4e597ab
b5da053
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we use ts ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh i see there are type errors as methods are undefined There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, good suggestion! I didn't even think of it. Let me try. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
import * as ReportUtils from '@libs/ReportUtils'; | ||
import * as Task from '@userActions/Task'; | ||
|
||
// There are some methods that are OK to use inside an action file, but should not be exported. These are typically methods that look up and return Onyx data. | ||
// The correct pattern to use is that every file will use it's own withOnyx or Onyx.connect() to access the Onyx data it needs. This prevents data from becoming stale | ||
// and prevents side-effects that you may not be aware of. It also allows each file to access Onyx data in the most performant way. More context can be found in | ||
// https://github.com/Expensify/App/issues/27262 | ||
describe('ReportUtils', () => { | ||
it('does not export getParentReport', () => { | ||
expect(ReportUtils.getParentReport).toBeUndefined(); | ||
}); | ||
}); | ||
|
||
describe('Task', () => { | ||
it('does not export getParentReport', () => { | ||
expect(Task.getParentReport).toBeUndefined(); | ||
}); | ||
}); |
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.
Seems we are intentionally doing this, but just mentioning...
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.
found this explaination justifiable 👌
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.
Thanks for pointing it out!