New hash algorithm#1920
Conversation
🦋 Changeset detectedLatest commit: 221ea6e The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for compiled-css-in-js canceled.
|
de65730 to
4ae94f9
Compare
kylorhall-atlassian
left a comment
kylorhall-atlassian
left a comment
There was a problem hiding this comment.
This looks good to me from here, but rollout and internal tooling is the hardest part — eg. internal tooling may be bound to 9-digit hashes in places via regex, but that should be an easy search for eslint or otherwise, eg. I personally may have used /_[a-z0-0]{8}/ a few times 🙈 but unclear.
This comment I raised for example https://github.com/atlassian-labs/compiled/pull/1920/changes#r3590535998 will happen both internally and in ecosystem for folks using pass-through styles (where we will have a mismatch of the collisionResistantHash, it's not if, it's when, as we're still going through issues with Emotion vs. Compiled 2 years on)
But it's the right change and it's really just more about comms and process.
| /** | ||
| * When `true`, atomic class names are generated with a collision-resistant | ||
| * hash: base-62 encoding, zero-padded to a fixed width, producing 11-character | ||
| * class names (`_` + 6-char group + 4-char value). When `false` (the default), | ||
| * the legacy base-36 hash truncated to 4 characters is used, producing | ||
| * 9-character class names — preserving existing output. | ||
| * | ||
| * This is a migration flag. It can be driven per-product by an environment | ||
| * variable or feature gate in the build config. Once every consumer has | ||
| * migrated, the default will flip to `true` and the legacy branch removed. | ||
| * | ||
| * The runtime `ax()` handles both class formats simultaneously (via | ||
| * length-based group-key extraction), so mixing old and new classes on a | ||
| * page is safe. | ||
| * | ||
| * @default false | ||
| */ | ||
| collisionResistantHash?: boolean; |
There was a problem hiding this comment.
Probably could use a website change as well; doesn't have to be a blocker or come in this PR.
This could be where we better document the migration path and possible conflicts with pass-through xcss to link in CHANGELOG / CDAC
| [ | ||
| 'should not cross-dedup legacy 9-char and new 11-char classes for the same property', | ||
| // During version-skew, ax() receives old 9-char classes from pre-built packages | ||
| // and new 11-char classes from the app's own styles in the same call. | ||
| // Different group key lengths mean they never falsely dedup each other. | ||
| // CLEANUP: remove this test when collisionResistantHash becomes the default | ||
| ['_syaz13q2', '_1UtDYzynoA'], | ||
| '_syaz13q2 _1UtDYzynoA', | ||
| ], |
There was a problem hiding this comment.
We could in theory handle this clash if we stick to base36 for both, but if we can find a way to roll this out, your approach here is best.
There will be conflicts via NPM on how this works though, here's an example:
import { Box } from '@atlaskit/primitives/compiled/box'; // `collisionResistantHash=false` coming from NPM (so you can't re-hash it)
import { cssMap } from '@atlaskit/css'; // `collisionResistantHash=true`
const styles = cssMap({
border: '1px solid red'
});
export default () => <Box xcss={styles.root} />This will get, in psuedo-code:
<style>
._abcd1234 { border: none }
._FeDcBa7890 { border: 1px solid red }
</style>
<div class="_abcd1234 _abcdef1234"></div>Now this works in isolation, but if atomic styles are ordered differently due to arbitrary file imports, things go awry.
The same can be said of the inverse—we'll need to cut a major when we distribute every Platform package with collisionResistantHash=true, but I expect the impact to be VERY MINIMAL as it only affects a minimal set of components…but such a release requires a http://community.developers.atlassian.com/ post and it will be hard to explain it to folks. I think this can be informational only, eg. "As of the next version, if you're using pass-through CSS like <Box xcss> or […], you may see styling conflicts and should opt into collisionResistantHash when bumping. See CHANGELOG for more details" — beyond just informing folks, no further process should be necessary — Tim Keir went through a similar, more complex process w/ TS v4 recently.
There was a problem hiding this comment.
Good pints @kylorhall-atlassian
need to cut a major when we distribute every Platform package with
collisionResistantHash=true
Yes, that is the plan
4ae94f9 to
8a4ca7e
Compare
8a4ca7e to
ae251de
Compare
Replace the base-36, 4-char-truncated atomic class name hash with a base-62 encoding of the full 32-bit MurmurHash2 value. - hashBase62(): new encoder sharing the murmur2 core with hash(); fixed-width, zero-padded, byte-identical to atlaspack's to_base62 - atomicify-rules: group hash 6 chars, value hash 4 chars (11-char classes) - ax(): extract group key via className.length - 4 (fast fixed-offset slice); old (9-char) and new (11-char) classes are length-disjoint so they never falsely deduplicate during migration - unit tests incl. an atlaspack cross-implementation parity guard Eliminates the ~93K effective hash space and leading-character bias that caused class name collisions and silent style drops in large apps. test: collapse multi-line ax() calls in transform helper Longer atomic class names (11 chars under the base-62 hash scheme) can push ax([...]) calls past prettier's default printWidth in the test formatter, causing them to wrap onto multiple lines. This wrapping is purely a cosmetic artifact of the test helper — the real emitted code is single-line. Collapse multi-line ax([...]) calls back to a single line after formatting so generated-code assertions stay stable regardless of class-name length. test(keyframes): update fixtures for base-62 class names test(css-prop): update fixtures for base-62 class names test(styled): update fixtures for base-62 class names test(css-map): update fixtures for base-62 class names test(class-names, xcss-prop): update fixtures for base-62 class names fix(ac, compress): update group hash length for base-62 6-char groups ac() and compress-class-names-for-runtime.ts hardcoded the atomic group hash length as 4 chars (5 with the leading underscore). With the new base-62 hash, the group is 6 chars (7 with the leading underscore). - ac.ts: import ATOMIC_GROUP_HASH_LENGTH from @compiled/utils instead of hardcoding 5; update isCompressed check from charCodeAt(5) to charCodeAt(ATOMIC_GROUP_LENGTH) - compress-class-names-for-runtime.ts: import ATOMIC_GROUP_HASH_LENGTH and use it in slice(1, ATOMIC_GROUP_HASH_LENGTH + 1) instead of hardcoded slice(1, 5) - ac.test.ts: update synthetic test data from 4-char (_aaaa) to 6-char (_aaaaaa) group patterns to match new format test(babel-plugin): update compress and hash-prefix fixtures for base-62 test(babel-plugin): update remaining __tests__ fixtures for base-62 class names test(strip-runtime, vite-plugin, react, css): update fixtures for base-62 class names fix(ac): define ATOMIC_GROUP_HASH_LENGTH locally to avoid @compiled/utils runtime dependency test(react, vite-plugin): update fixtures for base-62 class names
…ism migration flag - Thread collisionResistantHash through babel-plugin types → transform.ts → atomicify-rules.ts - atomicClassName branches: false = legacy base-36 9-char (unchanged default), true = base-62 11-char - Default to collisionResistantHash:true in test helpers (babel-plugin test-utils, strip-runtime transform.ts, root babel.config.json, css/vite-plugin test helpers) so fixtures document the new output format while shipped default remains legacy - Add legacy regression suite to atomicify-rules.test.ts proving default=false emits exact 9-char base-36 names unchanged - Add collision-resistance test to hash.test.ts proving base-62 eliminates collisions - Remove atlaspack cross-impl parity assertions (not required for correctness) - Remove inaccurate '~93K effective hash space' claim (actual bias is more complex) - Downgrade changeset from major to minor (additive opt-in, no breaking change)
Documents a confirmed collision pair (scrollbar-width vs text-anchor, group _1fjg) found in large production CSS bundles. Tests assert: 1. Legacy hash DOES produce group collisions on real CSS properties (the confirmed bug) 2. Base-62 6-char hash eliminates those collisions (the fix)
- ax: add legacy/new format dedup parity tests and cross-format no-dedup test to document correct behaviour during version-skew window. CLEANUP comments mark legacy-specific tests for removal when collisionResistantHash defaults to true. - atomicify-rules: rename cr -> collisionResistant, css -> cssDeclaration for clarity. Remove redundant third collision test case.
…+ new hash backward compat - ac() now locates the compression separator via indexOf and derives the uncompressed group via length - value hash, handling both the legacy 4-char group (9-char class) and the collision-resistant 6-char group (11-char class) - compress-class-names-for-runtime derives group length from the class instead of hardcoding it, keeping legacy class-name compression backward compatible - add legacy-format ac() cases and a dedicated compress helper unit test - remove migration-artifact comment; tighten vite extraction regex
ae251de to
072aded
Compare
3 participants
What is this change?
Adds an opt-in
collisionResistantHashoption to the Compiled babel plugin / transform that generates atomic class names using a base-62 full-width hash, eliminating hash collisions.Default is
false— output is byte-for-byte identical to today. This release is additive and non-breaking. Teams can opt in incrementally; a future major release flips the default and removes the legacy branch.Why are we making this change?
Current hash doesn't guarantee unique group hash, especially in large repos or libraries with several nested selectors.
When two rules collide on the group hash,
ax()deduplicates them as if they were the same property — silently dropping styles.How are we making this change?
packages/utils/src/hash.tshashBase62(str, length)— full 32-bit MurmurHash2 → base-62 (0-9a-zA-Z), zero-padded to fixed widthATOMIC_GROUP_HASH_LENGTH = 6andATOMIC_VALUE_HASH_LENGTH = 4constantshash()(base-36) unchanged — still used for keyframe names, CSS variable names, and asset filenames (deliberately, so filenames stay lowercase and avoid case-insensitive-filesystem collisions)packages/css/src/plugins/atomicify-rules.tscollisionResistantHash?: booleanoption (defaultfalse)true→_<6-char group><4-char value>(11-char class, e.g._3iDTPbvLZJ)false→ existing_<4-char group><4-char value>(9-char class) — unchangedpackages/react/src/runtime/ax.tsDone in #1924
Instead of slicing a fixed number of chars from the front, we slice everything except the last 4 (the value hash). Because the value hash is fixed-width (4 chars) in both formats, "everything before the value" is always exactly the full group key — regardless of how long the group is.
During migration, a single page can contain both legacy (from not-yet-rebuilt packages / pre-built npm CSS) and new (from rebuilt packages) classes. This is safe because the two formats produce group keys of different lengths:
_1e0c)_3iDTPb)A 5-char string and a 7-char string can never be equal, so a legacy class and a new class can never collide on the same map key — even if they represent the same CSS property. They coexist as separate entries; the CSS cascade (source order / specificity) resolves which wins, exactly as it would for any two unrelated atomic classes. There is no false deduplication across formats.
PR checklist
I have...
[x] Updated or added applicable tests
Because the class-name format changes when
collisionResistantHashis on, a large number of fixtures needed updating. This was done deliberately so the test suite documents the target (new) output, while a dedicated suite still guards the legacy path.Additional tests:
packages/react/src/runtime/__tests__/ax.test.ts,packages/utils/src/__tests__/hash.test.ts[x] Updated the documentation in
website/[x] Added a changeset (if making any changes that affect Compiled's behaviour)
Additionally to checklist
[x] apply same changes to Atlaspack SWC plugins -> will do in another PR, will also revisit the PR template so that we ensure things stay on sync