-
-
Notifications
You must be signed in to change notification settings - Fork 37
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
Unable to get desired style precedence without TssCacheProvider #115
Comments
Hi @ryancogswell how are you doing? Just have to wait the next MUI release and everything should be fixed. Best regards, |
Hi @garronej, |
I received the notification of the MUI release immediately after my last comment, so I was able to immediately try it out. I am still seeing my issue, but it may be more specific than what I first realized. The styles that I was noticing were with a usage of It turns out I can also reproduce this without tss-react in play at all. The same issue occurs using the
In the sandbox above, the second grid item displays the way I would want the first grid item to display (with the |
Oh no! See my patched |
Yes, I think that is the problem I'm seeing -- media query style rules from MUI are being injected after non-media query style rules from tss-react. I could restore the old behavior without you needing to reintroduce TssCacheProvider if |
Ok thank you for clarifying this for me.
It does already :) https://docs.tss-react.dev/update-to-v4#less-than-tsscacheprovider-greater-than-removed |
Well, this is good new for us, it will maybe help me convince @emotion and MUI that it's an issue worth addressing. Would you open an issue about it in the MUI repo and tag me? Best regards, |
What I am wondering is if this is happening only to some pepole or if everyone is experiencing this kind of issues but don't bother opening an issue about it. |
It doesn't support passing in a callback for the
I don't think this case changes the overall arguments in that thread. Changing the behavior in @emotion now would have too much impact on users dependent on the current behavior. |
Ok sorry I didn't read carefully enough. I am releasing an beta (4.2.0-beta.1) that implement your proposal.
Thank you for taking the time to read the thread. I worked a lot to make it possible that TSS shares the same cache with MUI, this was the whole point of V4. But because of this problem with media query I'm afraid this switch will cause more harm than good. What is your opinion on this? |
The application I work on doesn't use SSR (and I don't expect it to anytime in the next few years if ever), so I don't have an appreciation for the complexities the separate cache introduces for SSR. When troubleshooting styles in the browser developer tools, I prefer the effect of the separate cache where the styles from our customizations are completely separated from the MUI styles. I suspect for people starting from scratch, the edge cases are less of an issue because they will do what they need to do to get customizations to work and there aren't a lot of different MUI components with media query rules in the library's styles. The edge cases are a bigger issue for people migrating a large code base from JSS with MUI v4 to tss-react with MUI v5 where there isn't an easy way to find the places that may have issues (aside from visually checking everything where it would be easy to miss things). As far as the "official recommended" setup, probably go with the simpler route (no custom cache), but provide examples of both with a brief explanation of the potential reasons for using a custom cache for tss-react.
I'll try it out sometime in the next week or two. There isn't any urgency for me to upgrade tss-react to v4, I was just upgrading some packages that were going to force regression testing a lot of our app, so I decided to get other packages up-to-date at the same time. |
@ryancogswell thank you very much for taking the time to give me your insight on this. I leave the patch I published for you in beta until you confirm it fits your needs. Best regards, |
Was const ThemeProvider: FC<ThemeProviderProps> = ({
theme,
muiCacheKey = 'mui',
tssCacheKey = 'tss',
children,
}) => {
return (
<CacheProvider value={getMuiCache(muiCacheKey)}>
<TssCacheProvider value={getTssCache(tssCacheKey)}>
<MuiThemeProvider theme={theme}>
{children}
</MuiThemeProvider>
</TssCacheProvider>
</CacheProvider>
)
} I am not sure how I can pass |
I guess I can reintroduce it. I though it make more cense to provide the cache as a parameter when creating EDITl: Now I remember why I removed it, it's because it forces EDIT2: I guess I can document the fact that if you use |
I think if you use library X in your package, this library X should be in |
Yes of course but I mean, for example mui-datatables is using |
Ok, I understand now. |
I'm going to restore the provider |
@Semigradsky @ryancogswell I reintroduced |
@ryancogswell I did as you suggested https://docs.tss-react.dev/troubleshoot-migration-to-muiv5-with-tss I'll close this issue now. |
I was attempting to upgrade from v3 to v4, but had to revert it after being unable to achieve style ordering similar to v3 without the TssCacheProvider. For my v3 setup, I create a prepend cache for MUI and a non-prepend cache for tss-react. This ensures that the styles generated by
makeStyles
always win over MUI styles when CSS precedence is otherwise the same (something that was also true when using JSS'smakeStyles
though the mechanism for controlling the relative order was different). In the migration guide, I saw the example usingcreateMakeAndWithStyles
for this purpose, however that doesn't give me any way to vary the tss-react cache using context. This is important for me since I actually have two MUI caches and two tss-react caches -- one pair for ltr and one pair for rtl. These caches get swapped dynamically as the user changes the language of the page.Below are the relevant pieces for how I'm using v3:
Any recommendation on how to do this in v4?
The text was updated successfully, but these errors were encountered: