-
-
Notifications
You must be signed in to change notification settings - Fork 119
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
How to make every class name to be unique? #397
Comments
@Profesor08 See my reply for #387 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>
);
} |
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 One more example with different elements. You can't just extends one from another. And they both must change their styles depending on parent
|
@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? |
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. |
@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. 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.
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;
` |
Heyo! So the Line 31 in 168bbc5
|
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. |
@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"? |
Yes. If some prop will change, and after that will change back, we don't need to generate every time one new class name.
It will generate new class name on every render. |
@Profesor08 Okay, so caching at the style level still doesn't work because The current work around const Button = styled("div")`;;;;;`; //Random number of semicolons to force unique className A suggested fix
|
@Profesor08 Looks like it works as intended now assuming @cristianbote doesn't have any concerns with my PR. |
@cristianbote I opened PR #401 which reduces the bundle size and also fixes this issue. |
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 Agreed, per my last comment on the closed PR: 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")``; |
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 classgo11
, wtf?Some example code:
Unexpected result:
There is an additional repo with simple example: https://codesandbox.io/s/react-goober-forked-68nbz?file=/src/index.js
The text was updated successfully, but these errors were encountered: