-
Notifications
You must be signed in to change notification settings - Fork 3.2k
[Due for payment 2025-04-02] [$250] [Dev] Console Error: A props object containing a "key" prop is being spread into JSX #58261
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
Comments
Triggered auto assignment to @isabelastisser ( |
This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989 |
@isabelastisser I can reproduce this with the following steps: Precondition: Have saved search items. If you don't, create some.
Please assign this to me as per this Slack thread. |
|
ProposalPlease re-state the problem that we are trying to solve in this issue.We're seeing console errors on the Reports page stating "A props object containing a 'key' prop is being spread into JSX. What is the root cause of that problem?The SavedSearchMenuItem type includes a key property. When these menu items are mapped in the PopoverMenu component, all properties including key are spread into the JSX using {...menuItemProps}. React treats key as a special prop that shouldn't be passed to components, which causes the console error. App/src/components/PopoverMenu.tsx Line 304 in 80cbd29
App/src/components/Search/SearchPageHeader/SearchTypeMenuPopover.tsx Lines 32 to 37 in 80cbd29
What changes do you think we should make in order to solve the problem?We should keep the key property for identification purposes but prevent it from being passed as a prop to React components. There are 4 options:
type SavedSearchMenuItem = MenuItemWithLink & {
itemKey: string; // Changed from 'key' to 'itemKey'
hash: string;
query: string;
styles?: Array<ViewStyle | TextStyle>;
};
const renderedMenuItems = currentMenuItems.map(({ key, ...item }, menuIndex)
to this: {...Object.fromEntries(Object.entries(menuItemProps).filter(([key]) => key !== 'key'))}
const renderedMenuItems = currentMenuItems.map((item, menuIndex) => {
const {text, onSelected, subMenuItems, shouldCallAfterModalHide, key, ...menuItemProps} = item;
return (
<OfflineWithFeedback
key={key || `${item.text}_${menuIndex}`}
pendingAction={item.pendingAction}
>
<FocusableMenuItem
key={key || `${item.text}_${menuIndex}`}
pressableTestID={`PopoverMenuItem-${item.text}`}
title={text}
onPress={() => selectItem(menuIndex)}
// other props...
{...menuItemProps}
/>
</OfflineWithFeedback>
);
}); What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?We can write test to ensure the App/src/components/PopoverMenu.tsx Line 304 in 80cbd29
What alternative solutions did you explore? (Optional) |
ProposalPlease re-state the problem that we are trying to solve in this issue.Console Error: A props object containing a "key" prop is being spread into JSX. What is the root cause of that problem?
When adding saved search to menu items all props are passed
So when rendering Popover menu the App/src/components/PopoverMenu.tsx Line 304 in 80cbd29
What changes do you think we should make in order to solve the problem?I don't see any use of Additionally, there are two similar App/src/components/Search/SearchPageHeader/SearchTypeMenuPopover.tsx Lines 32 to 37 in 80cbd29
Lines 102 to 107 in 80cbd29
So we should DRY code and use type from What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?This is a typecast error so I think no test is required What alternative solutions did you explore? (Optional)NA Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job. |
Job added to Upwork: https://www.upwork.com/jobs/~021899899286639282267 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @ntdiary ( |
@isabelastisser You might have missed to assign me 😄 |
@daledah @Shahidullah-Muffakir Can you confirm if we can safely remove the |
App/src/pages/Search/SearchTypeMenu.tsx Line 159 in 925c0fe
Then in App/src/components/MenuItemList.tsx Line 78 in 925c0fe
App/src/components/MenuItemList.tsx Line 85 in 925c0fe
But both places already have fallback value to be I also ran all tests with the The prop was added in #56326. I don't see any place mentioned this, so I asked the author here |
@isabelastisser, can you please help unassign me? seems I can't unassign myself. :D |
Triggered auto assignment to @mollfpr, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
@Shahidullah-Muffakir proposal looks good to me! Assigning! |
PR is ready for review. |
Issue not reproducible during KI retests. (First week) |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.1.18-4 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2025-04-02. 🎊 For reference, here are some details about the assignees on this issue:
|
@dominictb @isabelastisser @dominictb The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button] |
BugZero Checklist:
Bug classificationSource of bug:
Where bug was reported:
Who reported the bug:
|
Payment Summary
BugZero Checklist (@isabelastisser)
|
Payment summary: Contributor @Shahidullah-Muffakir paid $250 via Upwork https://www.upwork.com/nx/wm/offer/106762783 |
@dominictb what's your Upwork profile? |
@isabelastisser My Upwork profile is https://www.upwork.com/freelancers/~01f70bed1934fd35d5. Could you please send an offer? TIA! |
@dominictb Whoops! This issue is 2 days overdue. Let's get this updated quick! |
@dominictb, I messaged you in Upwork. |
Uh oh!
There was an error while loading. Please reload this page.
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: v9.1.10-3 Develop
Reproducible in staging?: Needs Reproduction
Reproducible in production?: Needs Reproduction
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?:
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @QichenZhu
Slack conversation (hyperlinked to channel name): #Expensify Bugs
Action Performed:
Expected Result:
No console errors on the Reports page.
Actual Result:
Console Error: A props object containing a "key" prop is being spread into JSX.
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @dominictbThe text was updated successfully, but these errors were encountered: