Skip to content
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 errors around fewer/more hooks in Storybook #7870

Merged

Conversation

mgadewoll
Copy link
Contributor

@mgadewoll mgadewoll commented Jul 2, 2024

Summary

After merging the Storybook code-snippet addon feature and right after the EUIColorPicker Emotion conversion updates, an issue emerged in the code-snippet where Storybook previews render an error due to fewer rendered (conditional) hooks. (see an affected component here)

This is due to the functionality within the code-snippet addon that checks for the story's defaultProps and has to evaluate components where needed to get to the right react element (recursively checking children until the story element is found).

This can be fixed (and optimized) by making sure we only check for the default props for the story element. It's not needed to check for other elements, as they won't have default props and are not relevant to the code snippet.

QA

  • open the affected story in Storybook and verify the error is gone and the story and its code snippet render as expected

@mgadewoll mgadewoll added documentation Issues or PRs that only affect documentation - will not need changelog entries skip-changelog labels Jul 2, 2024
- this prevents potential errors around fewer/more hooks being rendered
@mgadewoll mgadewoll force-pushed the storybook/code-snippet-fewer-hook-fix branch from 4bbba27 to d2c2547 Compare July 2, 2024 17:16
@mgadewoll mgadewoll marked this pull request as ready for review July 3, 2024 07:02
@mgadewoll mgadewoll requested a review from a team as a code owner July 3, 2024 07:02
@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

Copy link
Member

@cee-chen cee-chen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much for catching this Lene! You're a rock star!

@mgadewoll mgadewoll merged commit 56a2f08 into elastic:main Jul 4, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Issues or PRs that only affect documentation - will not need changelog entries skip-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants