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

Chore: update documentation to address issue #10856 #12777

Merged
merged 3 commits into from
Jun 5, 2021
Merged

Conversation

jonniebigodes
Copy link
Contributor

With this pull request the issue #10856 is addressed and can be closed.

What was done:

  • Updated the workflows/stories-for-multiple-components page to address the issue at hand.
  • Updated the writing-stories/args to include a bit more context to match the page above.

Feel free to provide feedback

@jonniebigodes jonniebigodes added documentation patch:yes Bugfix & documentation PR that need to be picked to main branch labels Oct 14, 2020
Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

Mostly LGTM

The `children` `args` as any other arg needs to be JSON serializable. This means that you should:

- avoid using empty values.
- avoid using components that include third party libraries such as `emotion`.
Copy link
Member

Choose a reason for hiding this comment

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

not sure we know enough to put this in the docs. i think emotion does work in general, but fails in some cases--not sure which ones yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@shilman as you said it we don't know much about it yet, and it will be a bit of "pickle" (pardon the bad pun) to hunt down each and every use case, for now we could probably frame it as :

The `children` `args` as any other arg needs to be JSON serializable. This means that you should:
-  avoid using empty values
- use caution with components that include third party libraries

Let me know if you're ok with the changes and i'll commit them and we get it in the docs

@stale
Copy link

stale bot commented Dec 25, 2020

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

@shilman shilman merged commit 85426d9 into next Jun 5, 2021
@shilman shilman deleted the fix_issue_10856 branch June 5, 2021 03:07
@shilman shilman added the patch:done Patch/release PRs already cherry-picked to main/release branch label Jun 5, 2021
shilman added a commit that referenced this pull request Jun 5, 2021
Chore: update documentation to address issue #10856
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation patch:done Patch/release PRs already cherry-picked to main/release branch patch:yes Bugfix & documentation PR that need to be picked to main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants