zendeskgarden / react-components

:seedling: garden React components
https://zendeskgarden.github.io/react-components/
Apache License 2.0
1.1k stars 89 forks source link

refactor: remove `theme` `defaultProp` #1905

Closed ze-flo closed 2 months ago

ze-flo commented 2 months ago

Description

This initiates the process of removing defaultProps to avoid deprecation warnings, focusing first on theme. Since v9 depends entirely on ThemeProvider and its theme for dark mode, rendering a component without wrapping it in ThemeProvider should result in failure.

Detail

Checklist

ze-flo commented 2 months ago

Here's a table comparing popular React component libraries written with styled-components or emotion.js. All support theming and light / dark mode. The table highlights whether each library can render components without first nesting them inside a ThemeProvider.

Library Works without ThemeProvider Notes Docs Sandbox
@mui/material DefaultProps injected via React's context API with defaultPropsProvider https://mui.com/material-ui/getting-started/usage/ https://codesandbox.io/p/sandbox/jgtp7x
@mantine/core https://mantine.dev/guides/vite/#setup
@primer/react https://primer.style/guides/react
@chakra-ui/react https://v2.chakra-ui.com/getting-started
@adobe/react-spectrum Components requiring theme render null if its missing. Others render unstyled. https://react-spectrum.adobe.com/react-spectrum/getting-started.html#setting-up-your-app https://codesandbox.io/p/sandbox/admiring-wood-fqqj8h

While @mui/material has a built-in mechanism to handle the absence of a ThemeProvider, most tested libraries strictly require it to be set up for proper rendering and styling. The behavior varies across different libraries, with some failing to render components and others rendering unstyled components without a theme.

jzempel commented 2 months ago

@ze-flo I think this is pretty compelling evidence that you're on the right path with this PR. Can we get the migration.md updated with the breaking change to theming and either add a corresponding ADR or have a plan to add it immediately after?

ze-flo commented 2 months ago

@ze-flo I think this is pretty compelling evidence that you're on the right path with this PR. Can we get the migration.md updated with the breaking change to theming and either add a corresponding ADR or have a plan to add it immediately after?

I updated the migration docs and created a ticket to add the corresponding ADR in a follow-up PR.