The render sequence is wrong when passing CSS classes to child

[ghost]

Let's assume we have ComponentA and ComponentB in different files.

// file1.tsx
const useStyles = makeStyles()(({ palette, breakpoints }) => ({
  extra: {
    color: "red"
  }
}));

const ComponentA = () => {
  const { classes, cx } = useStyles();
  return <ComponentB extraClasses={classes.extra} />;
}

// file2.tsx
const useStyles = makeStyles()(({ palette, breakpoints }) => ({
  root: {
    color: "blue",
    [breakpoints.up("sm")]: {
      color: "yellow"
    }
  }
}));

export const ComponentB = ({ extraClasses }: { extraClasses?: string }) => {
  const { classes, cx } = useStyles();
  return <div className={clsx(classes.root, extraClasses)}>Hi</div>;
};

Ideally when using JSS, the extraClass in ComponentA will override the root class in ComponentB which can produce the right result on screen, which is that the text should be in "red" color.

Problems:

1. If using clsx to merge classes, then two classes will be generated but in wrong sequence. The root class in ComponentB will always override extraClass in componentA and display "blue" color which is wrong.

2. If using cx to merge classes, then one merged class will be generated in right sequence, but if there's any breakpoint related css in ComponentB, then it will not work well because the css priority is always breakpoint first, so it will display "yellow" color which is also wrong.

Thanks for building this amazing package and this is the only issue we have so far.

Hope you can provide a fix or let us know if there's any workaround. Our project is huge so finding them and changing them one by one is kind of impossible..

Cheers
CJ

[garronej]

Hi,
I understand the problem you are facing.
It's a tough problem because your use-case is very common yet TSS is working as intended and if I change the behavior I expect another issue to pop up. You are saying that this was working in JSS?
I will try to think of a solution but, in the meantime, let me offer you a workaround that explains why we didn't notice this "problem" before.

With TSS, if we aren't using SSR*, we can avoid using media queries altogether.
In onyxia-ui we implement the following approach:

*It's in my roadmap to make this approach compatible with SRR though.

theme.ts

import { useMemo } from "react";
import { useWindowInnerSize } from "powerhooks/useWindowInnerSize";
import { useTheme as useMuiTheme } from "@mui/material/styles";
import { createMakeAndWithStyles } from "tss-react/compat";

function useTheme() {

	// gives window.innerWidth, re-render when it changes.
	const { windowInnerWidth } = useWindowInnerSize();

	const muiTheme = useMuiTheme();

	const theme = useMemo(
		() => ({
			...muiTheme,
			windowInnerWidth
		}),
		[windowInnerWidth, muiTheme]
	);

	return theme;

}

export const breakpointsValues = {
	"sm": 600,
	"md": 960,
	"lg": 1280,
	"xl": 1920,
} as const;

export const { makeStyles, withStyles } = createMakeAndWithStyles({ useTheme });

ComponentB.tsx

import { makeStyles, breakpointsValues } from "./theme";

const useStyles = makeStyles()(({ windowInnerWidth }) => ({
	root: {
		color: (() => {

                        /*
			if (windowInnerWidth >= breakpointsValues.xl) {
				return "red";
			}
			if (windowInnerWidth >= breakpointsValues.lg) {
				return "cyan";
			}
                        */

			if (windowInnerWidth >= breakpointsValues.sm) {
				return "yellow";
			}

			return "blue";

		})()
	}
}));

export const ComponentB = ({ extraClasses }: { extraClasses?: string }) => {
	const { classes, cx } = useStyles();
	return <div className={cx(classes.root, extraClasses)}>Hi</div>;
};

We found that it's much easier to reason about responsive styles with this approach. It's also easier to debug.

If you want something that could be implemented by a codemod scrip you could write this:

ComponentB.tsx

import { makeStyles, breakpointsValues } from "./theme";

const useStyles = makeStyles()(({ windowInnerWidth }) => ({
	root: {
		color: "blue",
		...(windowInnerWidth >= breakpointsValues.sm ? {
			color: "yellow"
		} : {})
	}
}));

export const ComponentB = ({ extraClasses }: { extraClasses?: string }) => {
	const { classes, cx } = useStyles();
	return <div className={cx(classes.root, extraClasses)}>Hi</div>;
};

Best

[garronej]

@chengjiaapraamcos if the website in question is apraamcos.com.au forget about my approach. It's SSR.

[ghost]

@chengjiaapraamcos if the website in question is apraamcos.com.au forget about my approach. It's SSR.

Yes bro, it is SSR and hope there can be a proper fix... Would it be an easy thing to fix or it might affect too many code?

[garronej]

Would it be an easy thing to fix or it might affect too many code?

I think I can pull it off but it's a big feature that will require at least a day of work.
I prefer to think of it as a feature since it is currently working as expected... but we'd like TSS to perform some extra magic.
I wrote a test case that fails for this issue but I will leave it at that for now I have to get back to work. I'll try to get this done this week end.

In the meantime, I know it's less than ideal but you could use && to increase specificity

[ghost]

Would it be an easy thing to fix or it might affect too many code?

I think I can pull it off but it's a big feature that will require at least a day of work. I prefer to think of it as a feature since it is currently working as expected... but we'd like TSS to perform some extra magic. I wrote a test case that fails for this issue but I will leave it at that for now I have to get back to work. I'll try to get this done this week end.

In the meantime, I know it's less than ideal but you could use && to increase specificity

Tons of thanks mate, we will use the windowInnerWidth workaround for now and wait for the proper fix. && is not quite ideal as it needs to be added in too many places and also it could not work with the classes been passed to more than two hierarchies.

[garronej]

You are welcome 👍🏻
about useWindowInnerSize() don't use the implementation of powerhooks, it's not SSR compatible.
Use an implementation more like this one:

function useWindowSize() {

  const [windowSize, setWindowSize] = useState({
    //It's up to you to figure out what is the best default values here
    width: 0,
    height: 0,
  });

  useEffect(() => {

     //Only called on frontend

      function handleResize() {
        // Set window width/height to state
        setWindowSize({
          width: window.innerWidth,
          height: window.innerHeight,
        });
      }
    
      // Add event listener
      window.addEventListener("resize", handleResize);
     
      // Call handler right away so state gets updated with initial window size
      handleResize();
    
      // Remove event listener on cleanup
      return () => window.removeEventListener("resize", handleResize);

  }, []); // Empty array ensures that effect is only run on mount
  return windowSize;
}

[ghost]

You are welcome 👍🏻 about useWindowInnerSize() don't use the implementation of powerhooks, it's not SSR compatible. Use an implementation more like this one:

function useWindowSize() {

  const [windowSize, setWindowSize] = useState({
    //It's up to you to figure out what is the best default values here
    width: 0,
    height: 0,
  });

  useEffect(() => {

     //Only called on frontend

      function handleResize() {
        // Set window width/height to state
        setWindowSize({
          width: window.innerWidth,
          height: window.innerHeight,
        });
      }
    
      // Add event listener
      window.addEventListener("resize", handleResize);
     
      // Call handler right away so state gets updated with initial window size
      handleResize();
    
      // Remove event listener on cleanup
      return () => window.removeEventListener("resize", handleResize);

  }, []); // Empty array ensures that effect is only run on mount
  return windowSize;
}

Thanks for providing this proper workaround. It's very useful but we still need to change all the breakpoints to this way which is a bit impossible for us. It's good as a short term solution so far and we are still waiting for the proper fix. Tons of thanks for all your supports so far :)

[garronej]

Well understood,
If SSR is involve the workaround I suggested isn't really one.
I'm going to spend a couple of hours on it now. ( not sure it'll be enough though )

for the proper fix

Sorry to insist on the status of this issue but this is a feature request. A good feature request but a feature request still. TSS currently works as advertised.

Tons of thanks for all your supports so far :)

You are welcome 😊

[garronej]

I got a POC working but it still needs work for being production ready.

[ghost]

I got a POC working but it still needs work for being production ready.

Tons of thanks bro :)

[garronej]

Everything should work in v1.1.0

[ghost]

Everything should work in v1.1.0

Just tried, working like a charm! Tons of thanks and it's perfect now!

Amazing and tons of thanks for your help!!!

[garronej]

Thank you for you appreciation 😊