fix: unrecognized prop warning in form autosuggest #3003

syedsajjadkazmii · 2024-02-13T07:26:55Z

Description

We encountered an issue where the childChildren prop passed to the MenuItem component was flagged as an unrecognized prop to DOM (locally), leading to a console warning. Simultaneously, this warning caused the occurrence of duplicate API calls within our MFE (frontend-app-authn).

Resolution: We renamed the childChildren to childchildren and the warning stopped appearing and the duplicate API calls issue was resolved.

We are still unsure why we were having duplicate API calls due to this warning. Maybe React was trying to re-render due to invalid props? But we made sure that there was no issue with our implementation and it was due to the unrecognized prop. But we would like to understand the actual reason behind this if possible.

Console Warning screenshot.

Before

In this video, we can notice that we are getting single API calls until we click on the FormAutoSuggest field. On clicking the field, we get the console warning and all our API calls start firing twice.

Screen.Recording.2024-02-13.at.12.56.25.PM.mov

After

In this video, (after the fix) we can see that we are neither getting console warnings nor duplicate API calls. All the API calls are being fired once.

Screen.Recording.2024-02-13.at.12.54.33.PM.mov

Deploy Preview

https://deploy-preview-3003--paragon-openedx.netlify.app/

Merge Checklist

If your update includes visual changes, have they been reviewed by a designer? Send them a link to the Netlify deploy preview, if applicable.
Does your change adhere to the documented style conventions?
Do any prop types have missing descriptions in the Props API tables in the documentation site (check deploy preview)?
Were your changes tested using all available themes (see theme switcher in the header of the deploy preview, under the "Settings" icon)?
Were your changes tested in the example app?
Is there adequate test coverage for your changes?
Consider whether this change needs to reviewed/QA'ed for accessibility (a11y). If so, please add wittjeff and adamstankiewicz as reviewers on this PR.

Post-merge Checklist

Verify your changes were released to NPM at the expected version.
If you'd like, share your contribution in #show-and-tell.
🎉 🙌 Celebrate! Thanks for your contribution.

netlify · 2024-02-13T07:27:00Z

✅ Deploy Preview for paragon-openedx ready!

Name	Link
🔨 Latest commit	`da34673`
🔍 Latest deploy log	https://app.netlify.com/sites/paragon-openedx/deploys/65cb8e76479139000880333a
😎 Deploy Preview	https://deploy-preview-3003--paragon-openedx.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

codecov · 2024-02-13T07:37:47Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (2801667) 93.18% compared to head (da34673) 93.18%.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #3003   +/-   ##
=======================================
  Coverage   93.18%   93.18%           
=======================================
  Files         249      249           
  Lines        4342     4342           
  Branches     1036     1036           
=======================================
  Hits         4046     4046           
  Misses        292      292           
  Partials        4        4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

brian-smith-tcril

Thanks for pointing out this bug! I believe the intended behavior is to set children on the cloned element since the childChildren variable used to just have the name children

paragon/src/Form/FormAutosuggest.jsx

Lines 75 to 82 in d32e6d0

    
           return React.cloneElement(child, { 
        
             ...rest, 
        
             children, 
        
             'data-value': children, 
        
             onClick: (e) => handleItemClick(e, onClick), 
        
             id: menuItemId, 
        
             onFocus: () => handleMenuItemFocus(menuItemId), 
        
           });

I've left suggestions that resolved the warning for me when testing locally.

brian-smith-tcril · 2024-02-13T14:52:38Z

src/Form/FormAutosuggest.jsx

@@ -87,13 +87,13 @@ const FormAutosuggest = forwardRef(

    function getItems(strToFind = '') {
      let childrenOpt = React.Children.map(children, (child) => {
-        const { children: childChildren, onClick, ...rest } = child.props;
+        const { children: childchildren, onClick, ...rest } = child.props;


Suggested change

const { children: childchildren, onClick, ...rest } = child.props;

const { children: childChildren, onClick, ...rest } = child.props;

brian-smith-tcril · 2024-02-13T14:53:11Z

src/Form/FormAutosuggest.jsx

+          childchildren,
+          'data-value': childchildren,


Suggested change

childchildren,

'data-value': childchildren,

children: childChildren,

'data-value': childChildren,

syedsajjadkazmii · 2024-02-13T15:45:58Z

I've left suggestions that resolved the warning for me when testing locally.

Hi @brian-smith-tcril, thanks for the suggestions. I have updated the PR.

openedx-semantic-release-bot · 2024-02-14T05:50:41Z

🎉 This PR is included in version 22.1.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

zainab-amir approved these changes Feb 13, 2024

View reviewed changes

brian-smith-tcril requested changes Feb 13, 2024

View reviewed changes

fix: unrecognized prop warning in form autosuggest

da34673

syedsajjadkazmii force-pushed the sajjad/form-autosuggest-unrecognized-prop branch from b3aa9ca to da34673 Compare February 13, 2024 15:44

brian-smith-tcril approved these changes Feb 13, 2024

View reviewed changes

syedsajjadkazmii merged commit 7e4a81f into master Feb 14, 2024
10 checks passed

syedsajjadkazmii deleted the sajjad/form-autosuggest-unrecognized-prop branch February 14, 2024 05:38

openedx-semantic-release-bot added the released label Feb 14, 2024

syedsajjadkazmii mentioned this pull request Feb 14, 2024

chore: upgrade paragon to v22.1.1 openedx/frontend-app-authn#1162

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: unrecognized prop warning in form autosuggest #3003

fix: unrecognized prop warning in form autosuggest #3003

syedsajjadkazmii commented Feb 13, 2024 •

edited

netlify bot commented Feb 13, 2024 •

edited

codecov bot commented Feb 13, 2024 •

edited

brian-smith-tcril left a comment

brian-smith-tcril Feb 13, 2024

brian-smith-tcril Feb 13, 2024

syedsajjadkazmii commented Feb 13, 2024 •

edited

openedx-semantic-release-bot commented Feb 14, 2024

	return React.cloneElement(child, {
	...rest,
	children,
	'data-value': children,
	onClick: (e) => handleItemClick(e, onClick),
	id: menuItemId,
	onFocus: () => handleMenuItemFocus(menuItemId),
	});

	const { children: childchildren, onClick, ...rest } = child.props;
	const { children: childChildren, onClick, ...rest } = child.props;

fix: unrecognized prop warning in form autosuggest #3003

fix: unrecognized prop warning in form autosuggest #3003

Conversation

syedsajjadkazmii commented Feb 13, 2024 • edited

Description

Before

After

Deploy Preview

Merge Checklist

Post-merge Checklist

netlify bot commented Feb 13, 2024 • edited

✅ Deploy Preview for paragon-openedx ready!

codecov bot commented Feb 13, 2024 • edited

Codecov Report

brian-smith-tcril left a comment

Choose a reason for hiding this comment

brian-smith-tcril Feb 13, 2024

Choose a reason for hiding this comment

brian-smith-tcril Feb 13, 2024

Choose a reason for hiding this comment

syedsajjadkazmii commented Feb 13, 2024 • edited

openedx-semantic-release-bot commented Feb 14, 2024

syedsajjadkazmii commented Feb 13, 2024 •

edited

netlify bot commented Feb 13, 2024 •

edited

codecov bot commented Feb 13, 2024 •

edited

syedsajjadkazmii commented Feb 13, 2024 •

edited