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

feat: Allow ampersand suffixes #923

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

stephenh
Copy link
Contributor

@stephenh stephenh commented Jan 25, 2025

Description

Allows nested rules like:

".Group:hover &": { color: "white" },

Where the & comes at the end.

Output:

.Group:hover .a { color: "white" }

This effectively fixes #702 , and mimics the Tailwinds capability of styling a child based on "some parent" being hovered.

I definitely understand the "nested selectors are terrible", and we have ~very few in our codebase 😅 (that currently uses Emotion), but this "change my style based on my parent", particularly with a CSS-driven :hover (instead of tracking parent hover state in JS), seems like a valid usage.

Packages

  • fela
  • fela-utils

Versioning

Minor

Checklist

Quality Assurance

You can also run pnpm run check to run all 4 commands at once.

  • The code was formatted using Prettier (pnpm run format)
  • The code has no linting errors (pnpm run lint)
  • All tests are passing (pnpm run test)

Changes

If one of the following checks doesn't make sense due to the type of PR, just check them.

  • Tests have been added/updated
  • Documentation has been added/updated

Copy link

vercel bot commented Jan 25, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
fela ✅ Ready (Inspect) Visit Preview Jan 26, 2025 3:41pm

@stephenh stephenh changed the title Allow ampersand suffixes. Allow ampersand suffixes Jan 25, 2025
@stephenh stephenh changed the title Allow ampersand suffixes feat: Allow ampersand suffixes Jan 25, 2025
@stephenh stephenh marked this pull request as ready for review January 25, 2025 20:44
@stephenh
Copy link
Contributor Author

Huh; I thought this was a pretty straight-forward approach, but I'm getting test failures in rehydration:

image

That shows the & getting dropped; but if anything I thought it would "show up more" because I removed the normalizeNestedProperty(property) that used to be dropping the &, but now is leaving it in place, so it can later be swapped by the generateCSSSelector logic.

@stephenh
Copy link
Contributor Author

stephenh commented Jan 26, 2025

I reverted to a simpler approach, that doesn't use replace and instead uses a pseudoPrefix flag to mean "put the class name at the end of pseudo".

This passes the current test suite, b/c & is eagerly removed from the pseudo selector, and so not in the cache keys, and so the hydration test suite passes.

That said, the & at the end will still break rehydration b/c right now rehydration is able to look at a CSS rule, and use the DECL_REGEX to parse back out what the original specificity prefix, class name, psuedo, etc., are all merely by looking at the rule.

However the regexes currently assumes "the class name is at the beginning" (technically after the specificity prefix) and I'm not sure it's possible for the regex to handle "actually the class name may be at the beginning or end" / "before or after the pseudo selector", i.e.

  • & #id > .foo ~ bar (current rehydration test) would be .a #id > .foo ~ bar, but
  • #id > .foo ~ bar & (new rehydration test with trailing &) would be #id > .foo ~ bar .a

This could probably work by adding a data-fela-amp type hint to the nodes, so that rehydration could look it up around here:

    if (rehydrationIndex !== -1) {
      const type = node.getAttribute('data-fela-type') || ''
      const media = node.getAttribute('media') || ''
      const support = node.getAttribute('data-fela-support') || ''
      const css = node.textContent

I.e. a node.getAttribute("data-fela-amp") that returns "start" or "end" or null, and if it was start/end, the regex could be adjusted appropriately.

This doesn't seem very hard, and is precedence for "rehydration hints" in the data-fela-support attribute, so maybe that's the right way to go?

Dunno, personally I wouldn't need rehydration, so maybe getting "ampersand suffixes" supported in rehydration could be left as a todo?

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.

Can't style based on global parent class
1 participant