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

styled gets empty theme values #102

Closed
evertheylen opened this issue Sep 6, 2022 · 5 comments
Closed

styled gets empty theme values #102

evertheylen opened this issue Sep 6, 2022 · 5 comments
Labels
bug Something isn't working

Comments

@evertheylen
Copy link

evertheylen commented Sep 6, 2022

See here for a demo. In short, a component like this:

const StyledDemo = styled('div')(({ theme }) => {
  console.log('THEME IS (styled)', theme);
  ...
})

will show an empty-ish theme value with many undefined keys (e.g. typography, palette, shadows, ...). However, if I build an otherwise identical component with useTheme like this:

const UseThemeDemo = (props) => {
  const theme = useTheme();
  console.log('THEME IS (useTheme)', theme);
  ...
};

It will log an much more "full" theme value as I would expect. I do believe this is the behaviour we want. Here are both logs:

image

Interestingly, this bug seems to affect the second demo on the SUID docs on styled. The styled button does not appear because Uncaught TypeError: theme.palette is undefined.

I've tried different versions of SUID/Solid but for some reason I can't find a combination that works. I've also tried explicitly setting a theme with ThemeProvider but that doesn't change anything to the "empty" theme values. I've also tested this on both chrome and firefox.

@juanrgm
Copy link
Member

juanrgm commented Sep 6, 2022

That result is correct.

Look at the same example but with @mui/material.

https://stackblitz.com/edit/react-ehhmhi?file=demo.tsx

@evertheylen
Copy link
Author

I am confused now. Is the empty theme expected behaviour? Then why does the styled demo on the SUID docs not work? The only difference there is that the styled component is wrapped in a ThemeProvider. Even then, I get an empty theme.

So to reiterate:

  • The second demo of styled, as present on the docs, does not work in stackblitz. You can click the little lightning icon on the docs or open this stackblitz
  • That same second demo of styled does seem to work with older versions of SUID. I used the old but still running docs at next.suid.io which gave me this stackblitz
  • I updated my own example with a ThemeProvider, but I get almost the same result: the theme in styled does not contain my typography settings, while it does in useTheme.

I will admit I'm not entirely clear on how themes work, esp when only a few things are specified (e.g. in the example above I only specify typography.fontSize). I kind of expect it to "merge" with some default theme so that all possible keys have a sensible default, but that may not be expected behaviour. Regardless, I do believe the current behaviour is broken, seeing that a demo on the docs site is broken.

@juanrgm
Copy link
Member

juanrgm commented Sep 7, 2022

This is a issue with Vite in develop mode (vitejs/vite#3301). In the past I tried to fix it, but no luck.

The problem is that Vite reloads the context files when they are called from differents paths, so the theme context is recreated and its reference too.

Now I know how fix it, thanks for report it.

@juanrgm juanrgm added WIP Work in process bug Something isn't working and removed WIP Work in process labels Sep 7, 2022
@juanrgm
Copy link
Member

juanrgm commented Sep 7, 2022

Closed via c5f7ab0.

@juanrgm juanrgm closed this as completed Sep 7, 2022
@evertheylen
Copy link
Author

Thanks a lot! I will add "does the error only occur in vite develop mode" to my bug checklist :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants