Skip to content

Fixed a type error in the active-visitors hook test - #30345

Merged
9larsons merged 1 commit into
mainfrom
slars/fix-active-visitors-test-types
Aug 27, 2026
Merged

Fixed a type error in the active-visitors hook test#30345
9larsons merged 1 commit into
mainfrom
slars/fix-active-visitors-test-types

Conversation

@9larsons

Copy link
Copy Markdown
Contributor

apps/admin-x-framework failed test:types (tsc --noEmit): in use-active-visitors.test.ts, renderHook infers its Props type from initialProps, which made config non-optional — so the rerender({config: undefined}) that exercises the empty-site_uuid fallback didn't typecheck, and the package's pnpm test was red locally.

Widened the initialProps value to typeof statsConfig | undefined so the inferred Props match the render callback's optional parameter. Test behavior unchanged.

Verification: npx tsc --noEmit clean in the package; the hook suite passes 14/14.

@coderabbitai

coderabbitai Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: QUIET

Plan: Pro Plus

Run ID: 1bbadfc8-5323-4f9a-8142-fece91921bff

📥 Commits

Reviewing files that changed from the base of the PR and between f8179ce and a51b2b6.

📒 Files selected for processing (1)
  • apps/admin-x-framework/test/unit/hooks/use-active-visitors.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review.

📜 Recent review details
⏰ Context from checks skipped due to timeout. (13)
  • GitHub Check: Tinybird required tests passed or skipped
  • GitHub Check: Unit tests (Node 22.23.1)
  • GitHub Check: App Playwright Acceptance Tests (@tryghost/activitypub)
  • GitHub Check: Build E2E Public App Assets
  • GitHub Check: Admin tests - Chrome
  • GitHub Check: App Playwright Acceptance Tests (@tryghost/admin)
  • GitHub Check: Stripe fixture checks
  • GitHub Check: Build Docker Images
  • GitHub Check: Build Admin
  • GitHub Check: Lint
  • GitHub Check: Check migration integrity
  • GitHub Check: Check app version bump
  • GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (6)
Review Admin UI for existing Shade reuse, correct component layer, semantic

⚙️ CodeRabbit configuration file

Files:

  • apps/admin-x-framework/test/unit/hooks/use-active-visitors.test.ts
Review whether tests prove changed behaviour, meaningful error/edge paths, and

⚙️ CodeRabbit configuration file

Files:

  • apps/admin-x-framework/test/unit/hooks/use-active-visitors.test.ts
Review lens: "where does this data become trusted?"

⚙️ CodeRabbit configuration file

Files:

  • apps/admin-x-framework/test/unit/hooks/use-active-visitors.test.ts
Prioritise concrete correctness, security, data-integrity, compatibility,

⚙️ CodeRabbit configuration file

Files:

  • apps/admin-x-framework/test/unit/hooks/use-active-visitors.test.ts
Type-safe boundaries: Fail only if the PR:

📄 CodeRabbit inference engine (Custom checks)

Files:

  • apps/admin-x-framework/test/unit/hooks/use-active-visitors.test.ts
Always use `pnpm`, never npm or Yarn.

📄 CodeRabbit inference engine (AGENTS.md)

Files:

  • apps/admin-x-framework/test/unit/hooks/use-active-visitors.test.ts
🔇 Additional comments (1)
apps/admin-x-framework/test/unit/hooks/use-active-visitors.test.ts (1)

136-136: LGTM!


Walkthrough

The useActiveVisitors test now passes statsConfig to renderHook with the type typeof statsConfig | undefined. This matches the optional config parameter used by the render callback. The change affects TypeScript typing only and does not change runtime behavior.

Merge Risk: ⚪ Minimal · up to a51b2

This localized test-only change widens the inferred props type so the existing undefined-config case typechecks without changing hook behavior. The package typecheck and 14 hook tests pass, so no actionable merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 6
✅ Passed checks (6 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the TypeScript fix in the active-visitors hook test.
Description check ✅ Passed The description accurately explains the TypeScript error, the type widening change, and verification results.
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.
Type-Safe Boundaries ✅ Passed PASS. The PR changes one line in apps/admin-x-framework/test/unit/hooks/use-active-visitors.test.ts. The type assertion only widens test initialProps for renderHook. The custom check explicitly …
New Files Are Typescript ✅ Passed PASS: The pull request changes only the pre-existing TypeScript file apps/admin-x-framework/test/unit/hooks/use-active-visitors.test.ts. The exact parent-to-HEAD diff contains no added .js, .jsx
Full details: Type-Safe Boundaries

Explanation

PASS. The PR changes one line in apps/admin-x-framework/test/unit/hooks/use-active-visitors.test.ts. The type assertion only widens test initialProps for renderHook. The custom check explicitly excludes tests, and the PR changes no boundary-consuming production code.

Full details: New Files Are Typescript

Explanation

PASS: The pull request changes only the pre-existing TypeScript file apps/admin-x-framework/test/unit/hooks/use-active-visitors.test.ts. The exact parent-to-HEAD diff contains no added .js, .jsx, .cjs, or .mjs files.

✨ 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 slars/fix-active-visitors-test-types

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

@nx-cloud

nx-cloud Bot commented Aug 27, 2026

Copy link
Copy Markdown

🤖 Nx Cloud AI Fix

Ensure the fix-ci command is configured to always run in your CI pipeline to get automatic fixes in future runs. For more information, please see https://nx.dev/ci/features/self-healing-ci


View your CI Pipeline Execution ↗ for commit a51b2b6

Command Status Duration Result
nx run @tryghost/admin:test:acceptance ✅ Succeeded 8m 50s View ↗
nx run-many -t test:unit -p @tryghost/admin-x-f... ✅ Succeeded 3m 2s View ↗
nx run ghost-admin:test ✅ Succeeded 2m 51s View ↗
nx run ghost-monorepo:lint:boundaries ✅ Succeeded 23s View ↗
nx run @tryghost/admin:build ✅ Succeeded 2m 6s View ↗
nx run-many -t lint -p @tryghost/admin-x-framew... ✅ Succeeded 1m 52s View ↗
nx run @tryghost/activitypub:test:acceptance ✅ Succeeded 53s View ↗
nx run-many --target=build --projects=tag:publi... ✅ Succeeded <1s View ↗
nx run @tryghost/e2e:test:fixtures ✅ Succeeded <1s View ↗

💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗


☁️ Nx Cloud last updated this comment at 2026-08-27 12:54:59 UTC

@codecov

codecov Bot commented Aug 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.91%. Comparing base (f8179ce) to head (a51b2b6).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #30345   +/-   ##
=======================================
  Coverage   75.91%   75.91%           
=======================================
  Files        1671     1671           
  Lines      159291   159291           
  Branches    19362    19362           
=======================================
  Hits       120923   120923           
  Misses      37360    37360           
  Partials     1008     1008           
Flag Coverage Δ
admin-tests 57.27% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@9larsons
9larsons merged commit 1978999 into main Aug 27, 2026
54 checks passed
@9larsons
9larsons deleted the slars/fix-active-visitors-test-types branch August 27, 2026 13:12
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