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: change ThemeProvider's children to non-required #58

Merged
merged 1 commit into from
Jun 2, 2023

Conversation

wjq990112
Copy link
Contributor

In React, as a Provider, the children prop is always optional. Therefore, modify the children prop in ThemeProviderProps to be non-required.

@arvinxx
Copy link
Collaborator

arvinxx commented May 29, 2023

is there any usage to use ThemeProvider without children?

I thought there is no such usage so I make children props required.

@wjq990112
Copy link
Contributor Author

I understand your point of view that setting children as required ensures that the ThemeProvider always wraps a child component. However, in React, the children prop of a Provider is usually optional for the following reasons:

  1. Consistency: Other Provider components in React (such as the Provider in React-Redux) typically have children as optional. Following this convention makes it easier for developers to understand and use the ThemeProvider.

  2. Flexibility: Although in most cases we indeed want the ThemeProvider to wrap child components, there might be scenarios where one needs to dynamically add or remove child components at runtime. In such cases, having children as optional provides greater flexibility.

  3. Usability: Making children a non-required prop simplifies certain testing and experimentation scenarios. For example, during development, a developer might want to temporarily use the ThemeProvider without passing any child components. If children were required, this would result in unnecessary errors and extra work.

In conclusion, I suggest making the children prop of the ThemeProvider non-required. This will make the component more flexible, usable, and consistent with other React Provider components.

@arvinxx
Copy link
Collaborator

arvinxx commented Jun 2, 2023

Get it, thanks for your contrubution~

@arvinxx arvinxx merged commit 3c13c7d into ant-design:master Jun 2, 2023
github-actions bot pushed a commit that referenced this pull request Jun 2, 2023
## [3.2.2](v3.2.1...v3.2.2) (2023-06-02)

### 🐛 Bug Fixes

* fix ThemeProvider's children to non-required ([3c13c7d](3c13c7d)), closes [#58](#58)
@github-actions
Copy link

github-actions bot commented Jun 2, 2023

🎉 This PR is included in version 3.2.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants