Builder-Vite: Use djb2 hash to prevent variable name collisions in builder-vite#34274
Conversation
…ns with pnpm Replace the naive character-code-sum hash with djb2 to prevent collisions when pnpm peer dependency hash suffixes produce paths with identical sums. Closes storybookjs#34270 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughUpdated the internal hash used when generating preview annotation variable name suffixes: the implementation switched from summing character codes to a djb2-style rolling hash (starting at 5381) that returns an unsigned 32-bit integer, changing emitted identifier values in generated code. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code/builders/builder-vite/src/codegen-project-annotations.ts`:
- Around line 108-115: The djb2 implementation in function hash currently
applies >>> 0 only on return; change the loop so the accumulator is forced to
32-bit unsigned on each iteration by updating acc as ((acc << 5) + acc +
value.charCodeAt(i)) >>> 0 inside the for loop (use the acc variable and
value.charCodeAt(i) as shown) and keep the final return as acc (no extra >>> 0
needed).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dcead538-6884-4184-9e4f-0b56522b6cdf
📒 Files selected for processing (1)
code/builders/builder-vite/src/codegen-project-annotations.ts
Move the unsigned right shift inside the loop so the accumulator stays within 32-bit range on every iteration, avoiding precision loss when hashing long file paths (common with pnpm). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Thank you so much for your contribution! I am pasting some AI-generated explanation here for future reference: How the Fix WorksThe Problem: Position-Blind HashingThe old hash sums character codes without caring about order: In the pnpm case, package paths contain hex suffixes like: These paths are anagrams of each other (same hex characters, different order) → same hash → preview_19913 declared twice → Vite build error. The Fix: djb2 — Position Mattersdjb2 uses the recurrence: acc = acc * 33 + charCode Starting from seed 5381, each character multiplies the accumulator before adding the next character code. This means the same characters in different positions produce different outputs. Step-by-step for "ab":
Step-by-step for "ba":
"ab" → 5,860,568 vs "ba" → 5,860,600 — no collision. Why * 33 and seed 5381? Applied to the Real pnpm Case: Old hash: both paths contain the same hex characters → same sum → collision. djb2: D appearing at position 20 vs position 24 contributes completely different amounts to the final accumulator → different hashes → distinct variable names like preview_2847361 and preview_9134822. |
Package BenchmarksCommit: No significant changes detected, all good. 👏 |
…llision Builder-Vite: Use djb2 hash to prevent variable name collisions in builder-vite (cherry picked from commit 1c38a36)
Closes #34270
What I did
Replaced the naive character-code-sum hash function in
codegen-project-annotations.tswith the djb2 hash algorithm.Background
The existing implementation summed character codes without considering position:
This means different strings containing the same characters (anagrams) produce identical hashes.
With npm/yarn, preview paths are short and collisions don't occur. However, with pnpm, peer dependency hash suffixes (hex strings) are included in paths, causing different packages to produce the same hash value — resulting in duplicate
preview_XXXXvariable names and a build error:Fix
Replaced with djb2 hash (seed 5381,
hash * 33 + c), which incorporates character position into the computation, eliminating the collision.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
storybook buildThe symbol "preview_XXXX" has already been declarederror occursDocumentation
Summary by CodeRabbit