-
-
Notifications
You must be signed in to change notification settings - Fork 729
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
fix: prevent rendering too many hooks error #8667
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files |
const projectId = useRequiredPathParam('projectId'); | ||
const { total } = useActionableChangeRequests(projectId); | ||
|
||
const count = simplifyProjectOverview ? 0 : (total ?? 0); |
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.
Isn't if (!simplifyProjectOverview) return null;
a better check? If the flag is off we don't add empty elements to the DOM.
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.
No, because if we return null
, we won't render a label at all. This way, at least the label is the same, though we could potentially just use a different label
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.
Ok, I thought this was just the count. I'll go ahead and approve it then.
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.
But I found a nicer way to do it in ad7aa87. We'll just render the text. This makes it render exactly the same as all the other tabs do.
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.
LGTM
We found an issue where we'd get a minified react error referencing the LazyProjectExport component.
We suspect that the issue might be the conditional rendering of this component, so the fix is to always render it, but to use the flag to check whether we should show the count or not.