goober icon indicating copy to clipboard operation
goober copied to clipboard

How to make every class name to be unique?

Open Profesor08 opened this issue 2 years ago • 14 comments

I have a situation where I want to apply style to element on some case. But I can't target desired child element, because a lot of child elements has the same class name. .go11 is child of .go11 with doble class go11, wtf?

image

Some example code:

const WeatherWidget = styled<"div">(({ ...props }) => {
  <WidgetContainer {...props}>...</WidgetContainer>;
})``;

export const Weather = styled<"div">(({ ...props }) => {
  return (
    <Show when={store.showWeatherWidget}>
      <WeatherWidget {...props} />
    </Show>
  );
})``;

export const Header = styled("div")`
  display: grid;
  grid-template-columns: 30px 280px auto 1fr;
  align-items: start;
  gap: 32px;

  ${Weather.className} {
    grid-column: 2 / 4;
  }
`;

Unexpected result:

image

There is an additional repo with simple example: https://codesandbox.io/s/react-goober-forked-68nbz?file=/src/index.js

const ChildA = styled("div")``;

const ChildB = styled("div")``;

const Parent = styled("div")`
  &:hover {
    ${ChildA} {
      color: red;
    }

    ${ChildB} {
      color: blue;
    }
  }
`;

function App() {
  return (
    <Parent>
      <ChildA>ChildA</ChildA>
      <ChildB>ChildB</ChildB>
    </Parent>
  );
}

Profesor08 avatar Nov 30 '21 10:11 Profesor08

@Profesor08 See my reply for https://github.com/cristianbote/goober/issues/387 The current workaround is to refactor your code to move different hover values into the children instead of the parent. Ideally, you don't want two components to have the exact same css. Once their css is different, they can become their own components. Example: HoverLinkRed and HoverLinkBlue. If you need common css for both, you can extend existing components like the example below:

https://codesandbox.io/s/react-goober-forked-ynd84?file=/src/index.js

const StandardLink = styled("div")`
  background-color: green;
`;

const ChildA = styled(StandardLink)`
  &:hover {
    color: red;
  }
`;

const ChildB = styled(StandardLink)`
  &:hover {
    color: blue;
  }
`;

const Parent = styled("div")``;

function App() {
  return (
    <Parent>
      <ChildA>ChildA</ChildA>
      <ChildB>ChildB</ChildB>
    </Parent>
  );
}

B-Teague avatar Dec 04 '21 04:12 B-Teague

The current workaround is to refactor your code to move different hover values into the children instead of the parent.

This is required behavior. Some elements must change their styles on hovering their container. This is very common case, where you need to change something inside depending on hover. Showing/hiding some buttons, replacing one lement with onother. Some times they has different styles, but some times they have same styles, or just has some common wrapper.

Making them to extend some base styled do nothing, bug remains. Because hover targets their common className.

The one solution to make all this to work correctly is to assigin unique classname to element on styled() function call. Because every time function styled() is called, it creates an new component. And this new component is not the same as some component previously declared.

One more example with different elements. You can't just extends one from another. And they both must change their styles depending on parent :hover. There is one more bug. Styles priority are disrespected. purple color for link must have highter priority and remain purple.

const Link = styled("a")``;

const Button = styled("div")``;

const ControlWrapper = styled("div")`
  width: 100px;

  ${Link} {
    color: purple;
  }
`;

const LinkControl = styled(({ children }) => {
  return (
    <ControlWrapper>
      <Link>{children}</Link>
    </ControlWrapper>
  );
})``;

const Parent = styled("div")`
  &:hover {
    ${Link} {
      color: red;
    }
    ${Button} {
      color: blue;
    }
    ${ControlWrapper} {
      color: green;
    }
  }
`;

function App() {
  return (
    <Parent>
      <Link>ChildA</Link>
      <Button>ChildB</Button>
      <LinkControl>LinkControl</LinkControl>
    </Parent>
  );
}

Profesor08 avatar Dec 04 '21 13:12 Profesor08

@cristianbote There is definitely different behavior here. Just so we can compare apples and oranges, I cloned @Profesor08 example using styled-components instead of goober to compare functionality:

https://codesandbox.io/s/react-goober-forked-ynd84?file=/src/index.js

You can comment out the setup line and goober and enable styled-components to see how the color changes differently on hover.

@Profesor08 The LinkControl changes to red using styled-components. Were you expecting it to turn red or stay purple?

B-Teague avatar Dec 06 '21 03:12 B-Teague

So it looks like StyledComponents doesn't cache it's CSS or maybe only cache per component? Since every styled component has a unique class, this allows you to use the same CSS across multiple styled components without conflict.

B-Teague avatar Dec 06 '21 05:12 B-Teague

@Profesor08 A hacky workaround that I tested is to put in some dummy css to force the template strings to be different between Link and Button. This will generate unique class names which should address your problem. @cristianbote I think giving the developer the ability to force a unique class name when the css is the same might be a necessary feature to address this problem. Your thoughts?

I added this feature to morpho which fixed this exact problem. This will be more difficult to implement for goober since it's using tagged templates. Custom css class name prefixing: .article-header-xj38d

  • prefix - This option is used to specify the global custom prefix for generated css class names. Can be overridden by each css function call css({}, "customPrefix"). Default value is "morpho".
var key = prefix + rules.join("")
return cache[key] || (cache[key] = createStyle(rules, cssType, prefix))

Some possible options:

styled('button', 'myClassName')`
    border-radius: ${(props) => props.size}px;
`;

//Change css function signature?
css('myClassName')`
    border-radius: ${(props) => props.size}px;
`
css.prefix('myClassName')`
    border-radius: ${(props) => props.size}px;
`
cssPrefix('myClassName')`
    border-radius: ${(props) => props.size}px;
`

B-Teague avatar Dec 06 '21 05:12 B-Teague

Heyo!

So the go11 className comes from an empty style rule, which makes sense. I believe the community wanted this behaviour before as well so this is why this exists: https://github.com/cristianbote/goober/blob/168bbc55424ae3db95aca09c4fd02716efb98b78/src/styled.js#L31 would this be something that could help out?

cristianbote avatar Dec 07 '21 07:12 cristianbote

@cristianbote

So the go11 className comes from an empty style rule, which makes sense.

It is ok, if you manage only style rules, and you can continue doing it by this way. But this will not work for components. Every component is designed to be unique, and reusable. They all can have common styles, common classes, but every one must have his own unique class name. And all relations between components must use only their unique class names.

Profesor08 avatar Dec 07 '21 16:12 Profesor08

@cristianbote This makes total sense when you think about it like instances of an object class. Each styled component is its own class: Link, Button, Tab, etc.

You don't want Link and Button sharing the same css className. Each of the 75 instances of a Link on your website should only use the Link css className, and each of the 50 instances of the Button on your website should only use the Button css className.

This example should produce two separate classNames

const Button = styled("div")``;
const Link = styled("div")``;

<Parent>
   <Link className="go12345">text1</Link>
   <Button className="go6789">text2</Button>
   <Link className="go12345">text3</Link>
   <Button className="go6789">text4</Button>
</Parent>

Do we need a way to cache the css "per styled call"? What would happen if we just remove caching of css classNames?

B-Teague avatar Dec 08 '21 05:12 B-Teague

@B-Teague

Do we need a way to cache the css "per styled call"?

Yes. If some prop will change, and after that will change back, we don't need to generate every time one new class name.

What would happen if we just remove caching of css classNames?

It will generate new class name on every render.

Profesor08 avatar Dec 08 '21 06:12 Profesor08

@Profesor08 Okay, so

caching at the style level still doesn't work because Two components with the exact same styles will always generate the exact same hash and end up with the same className, so by using this library, no two components should have the exact same css (including styled components with empty template)

The current work around put in a random number of semicolons in the style to force unique css for all styled components

const Button = styled("div")`;;;;;`; //Random number of semicolons to force unique className

A suggested fix

  1. We want the hash function to still generate the same hash everytime css, keyframes, or glob is called to be backwards compatible
  2. The hash function has to create a new unique hash every time styled is called.
    One way to do this is an internal counter that increments every time style is called. (working on a PR now)

B-Teague avatar Dec 09 '21 01:12 B-Teague

@Profesor08 Looks like it works as intended now assuming @cristianbote doesn't have any concerns with my PR. https://codesandbox.io/s/react-goober-forked-3vnqd

B-Teague avatar Dec 09 '21 02:12 B-Teague

@cristianbote I opened PR #401 which reduces the bundle size and also fixes this issue.

B-Teague avatar Dec 10 '21 05:12 B-Teague

So, what is expected here? I'm leaning towards that this is an intended behaviour and outside of the scope of goober as I believe this is not doable from a runtime POV. Thoughts?

cristianbote avatar Feb 22 '22 19:02 cristianbote

@cristianbote Agreed, per my last comment on the closed PR: "I think we can state the library is working as designed unless you want to implement a way to generate unique class names for each time styled is called, but still preserve hydration for server side rendering."

Hydration would break if the order of Button1 and Button3 are switched if we used an internal counter. The only way to support this at runtime is evaluating the source code at runtime and using the name of the variable as part of the hashing algorithm which would just add bloat to the library with basically zero advantages.

const Button1 = styled("div")``;
const Button2 = styled("div")`color: red`;
const Button3 = styled("div")``;

B-Teague avatar Jun 14 '22 01:06 B-Teague