Skip to content

Changed skeleton component to use an integer-based key instead of a UUID#26745

Merged
troyciesco merged 2 commits intoTryGhost:mainfrom
cmraible:fix-skeleton-crypto-randomuuid
Mar 26, 2026
Merged

Changed skeleton component to use an integer-based key instead of a UUID#26745
troyciesco merged 2 commits intoTryGhost:mainfrom
cmraible:fix-skeleton-crypto-randomuuid

Conversation

@cmraible
Copy link
Copy Markdown
Collaborator

@cmraible cmraible commented Mar 9, 2026

no ref

When running Ghost in development with yarn dev in a remote VM, the analytics app crashes when loading over a plaintext http connection because crypto.randomUUID is only available in a secure context (https or localhost).

crypto.randomUUID is also overkill for React keys anyways, so this changes to using integers instead, to make it easier to develop Ghost in a remote VM environment.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 9, 2026

Walkthrough

The skeleton component in apps/shade/src/components/ui/skeleton.tsx was modified to change the React fragment key generation for skeleton items from skeleton-${crypto.randomUUID()} to skeleton-${i} where i is the loop index. No other logic, control flow, or public exports were changed.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description check ✅ Passed The PR description clearly explains the problem (crypto.randomUUID unavailable on non-HTTPS connections), the solution (replace with integer keys), and the rationale (improving remote VM development experience).
Title check ✅ Passed The title directly and accurately describes the main change: replacing UUID-based keys with integer-based keys in the skeleton component.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cmraible
Copy link
Copy Markdown
Collaborator Author

cmraible commented Mar 9, 2026

@CodeRabbit review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 9, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
apps/shade/src/components/ui/skeleton.tsx (1)

32-32: Use index-based keys for this fixed-order skeleton list.

Math.random() avoids the secure-context crash from crypto.randomUUID(), but React explicitly warns against generating keys "on the fly" because they won't match between renders and force unnecessary remounts. For a static-order placeholder list like skeleton rows, using the index as the key is stable and acceptable.

♻️ Suggested refactor
-            uniqueKeys.push(`skeleton-${Math.random().toString(36).substring(2, 15)}`);
+            uniqueKeys.push(`skeleton-${i}`);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/shade/src/components/ui/skeleton.tsx` at line 32, Replace the unstable
Math.random()-based keys with stable index-based keys for the fixed-order
skeleton list: instead of pushing `skeleton-${Math.random()...}` into
uniqueKeys, derive keys from the item's index (e.g., `skeleton-${index}`) or use
the index directly when mapping/rendering; update the code that builds
`uniqueKeys` and any rendering that uses those keys so they use the index-based
key for `uniqueKeys` to avoid remounts caused by changing random keys.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@apps/shade/src/components/ui/skeleton.tsx`:
- Line 32: Replace the unstable Math.random()-based keys with stable index-based
keys for the fixed-order skeleton list: instead of pushing
`skeleton-${Math.random()...}` into uniqueKeys, derive keys from the item's
index (e.g., `skeleton-${index}`) or use the index directly when
mapping/rendering; update the code that builds `uniqueKeys` and any rendering
that uses those keys so they use the index-based key for `uniqueKeys` to avoid
remounts caused by changing random keys.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 19fd7744-6b75-4de3-81be-c3895a5f1f14

📥 Commits

Reviewing files that changed from the base of the PR and between 2c5707b and 81d2b58.

📒 Files selected for processing (1)
  • apps/shade/src/components/ui/skeleton.tsx

@cmraible cmraible marked this pull request as ready for review March 18, 2026 03:34
cmraible added a commit that referenced this pull request Mar 24, 2026
…eton

ref #26745
Avoids secure-context crash on non-HTTPS and prevents unnecessary React remounts
@cmraible cmraible force-pushed the fix-skeleton-crypto-randomuuid branch from 5e1f326 to 82f5ee2 Compare March 24, 2026 18:32
@cmraible
Copy link
Copy Markdown
Collaborator Author

@CodeRabbit review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 24, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cmraible cmraible changed the title Fixed skeleton component crash on non-HTTPS connections Changed skeleton component to use an integer-based key instead of a UUID Mar 25, 2026
@cmraible cmraible requested a review from troyciesco March 25, 2026 17:07
@cmraible cmraible added the ok to merge for me You can merge this on my behalf if you want. label Mar 25, 2026
@cmraible
Copy link
Copy Markdown
Collaborator Author

@troyciesco no rush at all this, just a random little one I came across in my adventures

@troyciesco troyciesco merged commit 3740c0d into TryGhost:main Mar 26, 2026
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ok to merge for me You can merge this on my behalf if you want.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants