Classname hash string generator

[Dwlad90]

Hi @nmn, @necolas,

While working on the SWC plugin, I found that the variable names here were mixed up with their contents on lines 50-51.

stylex/packages/shared/src/convert-to-className.js

Lines 50 to 53 in 41fa0fa

	const atRuleHashString = sortedPseudos.join('');
	const pseudoHashString = sortedAtRules.join('');
	const modifierHashString = atRuleHashString + pseudoHashString || 'null';

Please tell me what should be first in the hash string: pseudo elements or media queries?

Thanks in advance for your answer

[nmn]

I found that the variable names here were mixed up with their contents on lines

Could you explain what this means?

---

At rules come first. Pseudo selectors come second. Both lists are sorted alphabetically.

NOTE: I recently updated the way pseudo selectors should be sorted in #740. You've highlighted a small edge-case bug here and I should update the sorting logic here as well. Currently there is a small edge-case where different selectors might generate the same hash. I will put up a PR soon.

[nmn]

I've created a new issue #749 and a PR to fix the issue. I didn't quite understand your question but hopefully the new logic makes sense.

[Dwlad90]

I found that the variable names here were mixed up with their contents on lines

Could you explain what this means?

At rules come first. Pseudo selectors come second. Both lists are sorted alphabetically.

NOTE: I recently updated the way pseudo selectors should be sorted in #740. You've highlighted a small edge-case bug here and I should update the sorting logic here as well. Currently there is a small edge-case where different selectors might generate the same hash. I will put up a PR soon.

Thanks for the answer, that's exactly what I needed.

I found this case just when I was working on the updated sorting logic from #740. When running one of the new tests, I got an incorrect class hash. After a little investigation, I managed to find out that the string for the hash differed in the order of pseudos and the atRules:

• StyleX version <>colorgreen:hover@media (min-width: 1000px)
• SWC version <>colorgreen@media (min-width: 1000px):hover.

However, right now, because of the confusion with variable assignments in your code, pseudos come first, that's the wrong order, right?

stylex/packages/shared/src/convert-to-className.js

Lines 50 to 53 in a55cd69

	const atRuleHashString = sortedPseudos.join('');
	const pseudoHashString = sortedAtRules.join('');
	const modifierHashString = atRuleHashString + pseudoHashString || 'null';

If this is indeed the case, can I open a PR with a fix of #749?

[nmn]

@Dwlad90 You're right! I have somehow missed this error for a whole year and didn't notice even when you mentioned it.

For the time being, we will keep pseudos first. I'll update my PR to fix the variable names.

In a separate commit, I'll try to put at-rules first as was intended, but not immediately.

[nmn]

I've updated #749 to fix the variable name to match the actual behaviour. Thanks again for catching this!

[Dwlad90]

You are welcome!