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

Remove automatic var(…) injection #13537

Merged
merged 3 commits into from Apr 18, 2024
Merged

Conversation

RobinMalfait
Copy link
Contributor

This PR removes automatic var(…) injection for arbitrary properties, values and modifiers.

There are a few properties that use "dashed-ident" values, this means
that you can use --my-thing as-is without the var(…) around it.

E.g.:

.foo {
  /* Notice that these don't have `var(…)` */
  view-timeline-name: --timeline-name; 
  anchor-name: --sidebar;
}

This causes issues because we are now injecting a var(…) where it's not needed.

One potential solution is to create a list of properties where dashed idents can be used. However, they can also use CSS custom properties that point to another dashed ident.

E.g.:

.foo {
  --target: --sidebar;
  anchor-name: var(--target);
}

A workaround that some people used is adding a _ in front of the variable: mt-[_--my-thing] this way we don't automatically inject the var(…) around it. This is a workaround and gross.

While the idea of automatic var injection is neat, this causes more trouble than it's worth. Adding var(…) explicitly is better.

A side effect of this is that we can simplify the internals for the candidate data structure because we don't need to track dashedIdent separately anymore.

There are a few properties that use "dashed-ident" values, this means
that you can use `--my-thing` as-is without the `var(…)` around it.

This causes issues because we are now injecting a `var(…)` where it's not
needed.

One potential solution is to create a list of properties where dashed
idents can be used. However, they can _also_ use CSS custom properties
that point to another dashed ident.

A workaround that some people used is adding a `_` in front of the
variable: `mt-[_--my-thing]` this way we don't automatically inject the
`var(…)` around it. This is a workaround and gross.

While the idea of automatic var injection is neat, this causes more
trouble than it's worth. Adding `var(…)` explicitly is better.

A side effect of this is that we can simplify the `candidate` data
structure because we don't need to track `dashedIdent` separately
anymore.
@adamwathan
Copy link
Member

Nice, think this is probably the right move. Before we merge this though we should prove we can write a codemod to upgrade people, maybe can do that early next cycle.

@RobinMalfait
Copy link
Contributor Author

@adamwathan yep, I already added this to our list of breaking changes so we don't forget. I'm sure there are more things we can write codemods for.

@RobinMalfait RobinMalfait merged commit cd4711c into next Apr 18, 2024
1 check passed
@RobinMalfait RobinMalfait deleted the remove-automatic-var-injection branch April 18, 2024 22:40
RobinMalfait added a commit that referenced this pull request May 8, 2024
RobinMalfait added a commit that referenced this pull request May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants