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

A mechanism to use css variable #3965

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

behnammodi
Copy link

@behnammodi behnammodi commented Mar 10, 2023

This PR related to Support CSS Varaible cause:

  • prevent any recalculation(branching) by styled-components for dynamic value
  • generates just 1 style rule by relying on CSS variables as a tool to ship value from the component to the rule
  • there is no breaking change

How can we use it?

To identify which prop is used for CSS variable purposes we can use $$ prefix, but it is not required.

const Comp = styled.div<{ $$fontSize?: string }>`      
  font-size: ${{ var: 'fontSize', compute: props => props.$$fontSize || "1rm" }};
`;

Usage:

<Comp $$fontSize="2rm" />

Then our component will be something like this:

<div className="sc-a b" style={{ "--fontSize": "2rm" }} />,

and it just generates a style rule even if we pass any dynamic values:

.b {
  font-size: var(--fontSize);
}

That's mean if I pass a new value:

<div className="sc-a b" style={{ "--fontSize": "50px" }} />,

It can still rely on the previous style rule and it doesn't need to create a new style rule branch

Without this PR

Without this PR if we want to do something like the above example we should move logic inside the style to the component:

Style:

const Comp = styled.div<{ $$fontSize?: string }>`      
  font-size: var(--fontSize);
`;

Component:

<Comp style={{ '--fontSize': props.fontSize || "1rm" }} />

Despite it using CSS variables, it creates a style rule branch per any new value for --fontSize

@quantizor
Copy link
Contributor

quantizor commented Apr 1, 2023

I wonder if synthesizing css variables dynamically to avoid re-injecting styles would have some perf wins… like making it more automatic than this method so you don’t have to manually define anything. 🤔

At the moment, interpolations in styles lead to style branching which injects a lot of extra CSS. Leveraging variables in common scenarios that don’t require branching (like just updating a primitive value) could drastically trim down generated CSS for many projects.

@behnammodi
Copy link
Author

behnammodi commented Apr 1, 2023

You don’t have to manually define anything. 

Then we have to move logics from separated styles file to component file:

Like this:

const Comp = styled.div`      
  font-size: var(--fontSize);
`;

<Comp style={{'--fontSize': props.fontSize || "1rm" }} />
Interpolations in styles lead to style branching which injects a lot of extra CSS

Exactly, but this PR can prevent any branching cause it relays on CSS variables and does not generate any other branch for dynamic values. please check this section:

propsForElement.style = varRules?.reduce(

@@ -154,6 +157,14 @@ function useStyledComponentImpl<Target extends WebTarget, Props extends Executio
}
}

propsForElement.style = varRules?.reduce(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I used Linaria I had an issue that style tag was too long when you have a lot of props and thus the DOM tree in devtools was almost unreadable. Maybe it is worth to insert css variables in style tag like we have for plain styles?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have any idea how?

for (let i = 0, len = chunk.length; i < len; i += 1) {
const next = chunk[i];
if (isVar(next)) {
result[i - 1] = result[i - 1] + 'var(--' + next.var + ')';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is one thing to think about - css variables accept default value, here you have no possibility to pass default value. However, I do not have ideas how to do that properly.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We always can have a default value in another way, please have a look at this example:

const Comp = styled.div<{ $$fontSize?: string }>`      
  font-size: ${{ var: 'fontSize', compute: props => props.$$fontSize || "1rm" }};
`;

Here 1rm is the default value as programmatically

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants