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

Use valid CSS selectors in useId format #32001

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sebmarkbage
Copy link
Collaborator

@sebmarkbage sebmarkbage commented Jan 7, 2025

For the useId algorithm we used colon : before and after. #23360

This avoids collisions in general by using an unusual characters. It also avoids collisions when concatenated with some other ID. Unfortunately, : is not a valid character in view-transition-name.

This PR swaps the format from:

:r123:

To the unicode:

«r123»

Which is valid CSS selectors. This also allows them being used for querySelector() which we didn't really find a legit use for but seems ok-ish.

That way you can get a view-transition-name that you can manually reference. E.g. to generate styles:

const id = useId();
return <>
  <style>{`
    ::view-transition-group(${id}) { ... }
    ::view-transition-old(${id}) { ... }
    ::view-transition-new(${id}) { ... }
  `}</style>
  <ViewTransition name={id}>...</ViewTransition>
</>;

Copy link

vercel bot commented Jan 7, 2025

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

Name Status Preview Comments Updated (UTC)
react-compiler-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 7, 2025 1:28am

@facebook-github-bot facebook-github-bot added the React Core Team Opened by a member of the React Core Team label Jan 7, 2025
sebmarkbage added a commit to sebmarkbage/react that referenced this pull request Jan 7, 2025
@react-sizebot
Copy link

react-sizebot commented Jan 7, 2025

Comparing: 3314162...39fccb5

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.js = 6.68 kB 6.68 kB = 1.83 kB 1.83 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 513.40 kB 513.42 kB = 91.72 kB 91.73 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.69 kB 6.69 kB = 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 518.18 kB 518.20 kB +0.01% 92.57 kB 92.58 kB
facebook-www/ReactDOM-prod.classic.js = 595.45 kB 595.47 kB = 104.81 kB 104.82 kB
facebook-www/ReactDOM-prod.modern.js = 585.72 kB 585.74 kB = 103.25 kB 103.26 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 4ee2293

@sophiebits
Copy link
Collaborator

Can/should we just make them fully alphanumeric?Saying “it’s guaranteed to be a valid CSS identifier” in the docs feels kinda random. Why only CSS? What if I want to put it in a shell command or HTML attribute or whatever? Is that guaranteed to be safe?

In principle someone should still be using CSS.escape anyway because you should always escape at the place where you are building up the string but I get that people won’t do that.

I think the letters WXYZ might currently be unused? And that still leaves underscore if people want to use that as their own delimiter.

@sebmarkbage
Copy link
Collaborator Author

It's not so much about the escaping as it's not allowed characters. CSS.escape isn't sufficient neither.

I'm not sure what alphanumeric characters we could that aren't high risk of collision unless they're a very long sequence. Especially since now this is a breaking change regardless and breaking it into a frequent collision seems worse than always breaking some edge case.

@sophiebits
Copy link
Collaborator

sophiebits commented Jan 7, 2025

Hmm. I guess the angle quotes are valid identifier chars in CSS but I don't think they are in most languages. Maybe safer to do a non-ASCII letter character like ɹ (used to write the pronunciation of "react" in IPA) or something. Then we could guarantee that it's unicode alphanumeric and reasonably unlikely to collide with other idents?

@eps1lon
Copy link
Collaborator

eps1lon commented Jan 7, 2025

#26839 has some more use cases. Specifically, anchor-name came up which does work with «»: https://codesandbox.io/p/sandbox/happy-heyrovsky-k4qzcg?file=%2Fsrc%2Findex.js%3A7%2C58

sebmarkbage added a commit to sebmarkbage/react that referenced this pull request Jan 8, 2025
@sebmarkbage
Copy link
Collaborator Author

Hmm. I guess the angle quotes are valid identifier chars in CSS but I don't think they are in most languages. Maybe safer to do a non-ASCII letter character like ɹ (used to write the pronunciation of "react" in IPA) or something. Then we could guarantee that it's unicode alphanumeric and reasonably unlikely to collide with other idents?

It's not in ASCII 7. The downside of using a unicode one is that it cannot be represented at all in a latin-1 encoding which seems even riskier.

@sophiebits
Copy link
Collaborator

Not sure why latin1 chars are less risky than unicode in general… did you have something specific in mind?

@sebmarkbage
Copy link
Collaborator Author

I mean you're not supposed to send it to the server or anything but if you sent it to a different document (like an unknown language then you'd lose it in the conversion to latin-1). It's kind of arbitrary though since the only languages that matter are on the client - which also means « should be fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants