Skip to content

test(demo): replace the bundle grep with a real single-React test - #208

Merged
Eliav2 merged 3 commits into
mainfrom
test/demo-single-react
Aug 8, 2026
Merged

test(demo): replace the bundle grep with a real single-React test#208
Eliav2 merged 3 commits into
mainfrom
test/demo-single-react

Conversation

@Eliav2

@Eliav2 Eliav2 commented Aug 8, 2026

Copy link
Copy Markdown
Owner

Follow-up to #203.

The problem with the check that shipped

grep -qF "$root_react" examples/dist/assets/*.js

It greps a minified bundle for a version string, so any unrelated occurrence of 19.2.0 is a false positive, and a duplicate React that happens to share the demo's version is a false negative. It is also guarded by [ "$demo_react" != "$root_react" ], so the matching-version case is never checked at all. And it only runs in CI, so the failure it guards cannot be reproduced locally.

What replaces it

Two tests in examples/, run through examples/vite.config.ts so they exercise the same alias and dedupe the dev server and production build use:

  • render an arrow
  • call the library's useXarrow from a component the demo compiled, which crosses the library/demo boundary even when rendering alone would survive

Root vitest cannot cover this: it resolves React from the repo root and never goes through the demo's config.

Proof it is not vacuous

With resolve.dedupe removed, both fail with the exact production error:

TypeError: Cannot read properties of null (reading 'useRef')
 Test Files  1 failed (1)
      Tests  2 failed (2)

Restored, both pass. Verified again after the vitest major bump below.

Two incidental fixes

examples uses vitest 4, not 3. Importing defineConfig from vitest/config on vitest 3 pulls Vite 7 types alongside @vitejs/plugin-react's Vite 8, which breaks tsc --noEmit and therefore examples build. vitest 4 accepts Vite 8.

eslint now ignores .worktrees/**. It is gitignored so CI never saw it, but locally eslint walked the built assets inside worktrees and reported 9161 errors, which would hide any real one. Dropped the now-dead webpack.*.js ignore while there.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Tests

    • Added integration tests for the example project, including arrow rendering and React instance compatibility.
    • Configured a browser-like test environment for component testing.
  • Chores

    • Added test and watch commands for the example project.
    • Updated linting exclusions and CI validation to run the example tests.

The check added in #203 grepped the minified demo bundle for the repo root's
React version string. That is unreliable in both directions: any unrelated
occurrence of the version is a false positive, and a duplicate React that
happens to match the demo's version is a false negative. It also only ran in
CI, so the failure it guards could not be reproduced locally.

Replaces it with two tests in examples/, run through examples/vite.config.ts so
they exercise the same alias and dedupe as the dev server and the production
build. One renders an arrow, the other calls the library's hook from a
component the demo compiled.

Verified they actually catch the bug: with resolve.dedupe removed both fail
with the exact production error, "Cannot read properties of null (reading
'useRef')", and pass with it restored.

Also:
- examples uses vitest 4, not 3. Importing defineConfig from vitest/config on
  vitest 3 pulls Vite 7 types alongside @vitejs/plugin-react's Vite 8 and
  breaks `tsc --noEmit`. vitest 4 accepts Vite 8.
- eslint now ignores .worktrees/**. It is gitignored so CI never saw it, but
  locally eslint walked the built assets inside worktrees and reported 9161
  errors. Dropped the now-dead webpack.*.js ignore at the same time.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@codesandbox

codesandbox Bot commented Aug 8, 2026

Copy link
Copy Markdown

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

@netlify

netlify Bot commented Aug 8, 2026

Copy link
Copy Markdown

Deploy Preview for react-xarrows ready!

Name Link
🔨 Latest commit 5d498a1
🔍 Latest deploy log https://app.netlify.com/projects/react-xarrows/deploys/6a777a55ebdc1c0008988b2e
😎 Deploy Preview https://deploy-preview-208--react-xarrows.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@Eliav2, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 43 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7f76dbf5-4c1e-48bc-a27f-8906eb2b5012

📥 Commits

Reviewing files that changed from the base of the PR and between 20aa04d and 5d498a1.

📒 Files selected for processing (2)
  • examples/src/__tests__/setup.ts
  • examples/vite.config.ts
📝 Walkthrough

Walkthrough

The example project now uses Vitest and Testing Library to test shared React behavior. CI runs the example tests. The previous duplicate-React bundle check was removed, and ESLint now ignores .worktrees/**.

Changes

Demo testing

Layer / File(s) Summary
Vitest setup and React integration tests
examples/package.json, examples/vite.config.ts, examples/src/__tests__/single-react.test.tsx
The example project adds Vitest scripts, jsdom configuration, testing dependencies, and tests for shared React behavior.
CI test validation and lint scope
.github/workflows/ci.yml, eslint.config.mjs
CI runs the example test suite and removes the duplicate-React bundle check. ESLint ignores .worktrees/**.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes replacing the bundle grep with a real single-React test, which is the main change.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch test/demo-single-react

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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 `@examples/vite.config.ts`:
- Around line 30-33: Add a Vitest setup file for the example tests that imports
cleanup from `@testing-library/react` and afterEach from vitest, then registers
afterEach(cleanup). Reference this setup file from the test configuration near
the existing environment, globals, and include settings so rendered Xarrow and
Xwrapper instances are cleaned up after every test.
🪄 Autofix

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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 04411491-6f4b-40a9-9321-c7ea2580ced8

📥 Commits

Reviewing files that changed from the base of the PR and between 15afe2b and 20aa04d.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (5)
  • .github/workflows/ci.yml
  • eslint.config.mjs
  • examples/package.json
  • examples/src/__tests__/single-react.test.tsx
  • examples/vite.config.ts

Comment thread examples/vite.config.ts
Review feedback on this PR.

Testing Library only registers its own cleanup when Vitest exposes afterEach as
a global, and this config keeps globals: false, so rendered trees accumulated in
the document.

That is not only untidy here. Xarrow resolves start and end with
document.getElementById, which returns the first match, so a leftover element
from an earlier test would be measured instead of the current one. Confirmed
with a probe: the second test saw 2 elements with id "from", and 1 after the
fix.

Re-verified the suite still fails with resolve.dedupe removed, so the added
cleanup did not weaken what these tests catch.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@Eliav2
Eliav2 merged commit a12128c into main Aug 8, 2026
6 checks passed
@Eliav2
Eliav2 deleted the test/demo-single-react branch August 8, 2026 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant