chore(repo): add react-grab to clerk-js sandbox#8666
Conversation
Loads react-grab from unpkg in the sandbox template so component locations can be inspected during local development. The sandbox has no production build, so the script is intrinsically dev-only.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 2b8d4cb The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types 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 |
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThis PR updates the clerk-js sandbox template and runtime: adjusts root/head theme variables, initializes and persists a sidebar-collapsed state via localStorage with desktop slide/transition and mobile visibility CSS, loads a dev react-grab script, refactors navigation into updated nav-link and new nav-group custom elements, adds desktop expand/collapse controls, and updates sandbox app appearance/variables initialization and reset. It also adds three new changeset markdown files. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/clerk-js/sandbox/template.html (1)
39-43: ⚡ Quick winPlease add a sandbox test for the React Grab integration path.
No tests were added/updated in this PR. A small integration assertion that the sandbox loads with the React Grab script present would cover this change.
As per coding guidelines "
**/*: If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/clerk-js/sandbox/template.html` around lines 39 - 43, Add an integration test that starts the sandbox (the dev:sandbox template) and asserts the rendered HTML contains the React Grab script tag (src="//unpkg.com/react-grab/dist/index.global.js" and crossorigin="anonymous"); locate the sandbox template referenced by the template.html change and create a test (e.g., "sandbox.react-grab.spec") that requests the sandbox page or loads the built template and checks for a script element with that src and attribute so this change is covered by CI.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/clerk-js/sandbox/template.html`:
- Around line 39-43: Update the protocol-relative, unversioned CDN import in the
sandbox template to an explicit HTTPS URL with a fixed version by replacing the
script src in packages/clerk-js/sandbox/template.html with
https://unpkg.com/react-grab@0.1.37/dist/index.global.js; then add a
sandbox/integration test that boots the dev sandbox and asserts the global
ReactGrab is present (e.g., a test that loads the template and checks
window.ReactGrab or similar), ensuring the test verifies the sandbox wiring for
the React Grab script.
---
Nitpick comments:
In `@packages/clerk-js/sandbox/template.html`:
- Around line 39-43: Add an integration test that starts the sandbox (the
dev:sandbox template) and asserts the rendered HTML contains the React Grab
script tag (src="//unpkg.com/react-grab/dist/index.global.js" and
crossorigin="anonymous"); locate the sandbox template referenced by the
template.html change and create a test (e.g., "sandbox.react-grab.spec") that
requests the sandbox page or loads the built template and checks for a script
element with that src and attribute so this change is covered by CI.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 27a23de7-ca4c-4370-bbf1-6d5c48e52672
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
.changeset/old-months-grow.mdpackages/clerk-js/package.jsonpackages/clerk-js/sandbox/template.html
| <!-- React Grab (development-only: this template is only used by `pnpm dev:sandbox`) --> | ||
| <script | ||
| src="//unpkg.com/react-grab/dist/index.global.js" | ||
| crossorigin="anonymous" | ||
| ></script> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify current dependency declaration vs sandbox CDN URL
jq -r '.devDependencies["react-grab"]' packages/clerk-js/package.json
rg -n "react-grab|unpkg.com/react-grab" packages/clerk-js/sandbox/template.htmlRepository: clerk/javascript
Length of output: 127
🏁 Script executed:
#!/bin/bash
# 1) Check what files changed (and whether any test-related files are among them)
echo "=== Changed files ==="
git diff --name-only
echo
echo "=== Changed test-related files ==="
git diff --name-only | rg -n -i '(^|/)(test|__tests__|tests|spec|cypress|playwright)/|\.test\.(js|ts|tsx|jsx)$|\.spec\.(js|ts|tsx|jsx)$' || true
echo
# 2) Confirm all unpkg references in the template
echo "=== unpkg.com references in template.html ==="
rg -n "unpkg\.com" packages/clerk-js/sandbox/template.html || true
echo
# 3) Show surrounding context for the snippet
echo "=== Context around CDN script tag ==="
sed -n '25,60p' packages/clerk-js/sandbox/template.htmlRepository: clerk/javascript
Length of output: 1728
Pin the React Grab CDN script URL (explicit HTTPS + version) and add sandbox test coverage.
packages/clerk-js/sandbox/template.htmlloads//unpkg.com/react-grab/dist/index.global.js(protocol-relative, unversioned); pin to an explicithttps://unpkg.com/react-grab@0.1.37/dist/index.global.js(or another exact version).- No tests were added/modified in this PR scope; add an integration/sandbox test that asserts React Grab loads correctly and sandbox wiring works.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/clerk-js/sandbox/template.html` around lines 39 - 43, Update the
protocol-relative, unversioned CDN import in the sandbox template to an explicit
HTTPS URL with a fixed version by replacing the script src in
packages/clerk-js/sandbox/template.html with
https://unpkg.com/react-grab@0.1.37/dist/index.global.js; then add a
sandbox/integration test that boots the dev sandbox and asserts the global
ReactGrab is present (e.g., a test that loads the template and checks
window.ReactGrab or similar), ensuring the test verifies the sandbox wiring for
the React Grab script.
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
| "bundlewatch": "^0.4.2", | ||
| "jsdom": "26.1.0", | ||
| "minimatch": "^10.2.5", | ||
| "react-grab": "^0.1.37", |
There was a problem hiding this comment.
since your loading from unpkg, whats the reason for needing it as devDependencies?
There was a problem hiding this comment.
no probably not. i didn't catch that. i'll remove from dev dep
The sandbox loads react-grab as a global script from unpkg in template.html, so the npm package isn't imported anywhere. Removing the devDependency also drops its transitive tree (bippy, ora, etc.) from the lockfile.
|
Snapi: no API changes detected in |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/clerk-js/sandbox/template.html (1)
291-311: ⚡ Quick winAdd test coverage for sidebar collapsed-state persistence.
This PR adds behavior (localStorage read/write +
data-sidebar-collapsedtoggling) but no tests were added/updated. Please add a sandbox-focused test that verifies initial state hydration from storage and collapse/expand state persistence.As per coding guidelines,
**/*: "If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/clerk-js/sandbox/template.html` around lines 291 - 311, Add sandbox-focused tests that cover the new sidebar collapsed-state persistence: write tests that (1) set localStorage[STORAGE_KEY] before DOMContentLoaded and assert that the HTML element has the data-sidebar-collapsed attribute after the page/script initializes (hydration), and (2) simulate clicks on the elements with IDs 'sidebarCollapse' and 'sidebarExpandBtn' and assert localStorage changes and that setCollapsed correctly toggles the data-sidebar-collapsed attribute. Reference the STORAGE_KEY constant and the setCollapsed function (and the click targets 'sidebarCollapse' and 'sidebarExpandBtn') when locating behavior to test; reset/cleanup localStorage and DOM between tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/clerk-js/sandbox/template.html`:
- Around line 291-311: Add sandbox-focused tests that cover the new sidebar
collapsed-state persistence: write tests that (1) set localStorage[STORAGE_KEY]
before DOMContentLoaded and assert that the HTML element has the
data-sidebar-collapsed attribute after the page/script initializes (hydration),
and (2) simulate clicks on the elements with IDs 'sidebarCollapse' and
'sidebarExpandBtn' and assert localStorage changes and that setCollapsed
correctly toggles the data-sidebar-collapsed attribute. Reference the
STORAGE_KEY constant and the setCollapsed function (and the click targets
'sidebarCollapse' and 'sidebarExpandBtn') when locating behavior to test;
reset/cleanup localStorage and DOM between tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: b55de652-09da-4e74-9141-e20821343645
📒 Files selected for processing (2)
.changeset/sandbox-sidebar-refresh.mdpackages/clerk-js/sandbox/template.html
✅ Files skipped from review due to trivial changes (1)
- .changeset/sandbox-sidebar-refresh.md
alexcarpenter
changed the title
Co-authored-by: Kyle MacDonald <kyle@clerk.dev>
3 participants
Description
Adds
react-grabto theclerk-jssandbox so component locations can be jumped to from the rendered Clerk UI during local development. Installed as adevDependencyon@clerk/clerk-jsand loaded insandbox/template.htmlvia the unpkg global script. The sandbox has no production build (onlypnpm dev:sandbox), so the tag is intrinsically development-only.To test, run
pnpm dev:sandbox, open http://localhost:4000, and use the default React Grab activation key on any rendered Clerk component.Checklist
Type of change