Sprint 5: Bindings & Interactivity — migrate React hooks to UI#70
Merged
pipewrk merged 7 commits intosprint/5-move-hooks-to-uifrom Oct 7, 2025
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR migrates React-facing hooks (useKernel, usePolicy, and resource hooks) from @geekist/wp-kernel to @geekist/wp-kernel-ui, making the kernel package transport-agnostic while consolidating UI integrations into a single package.
Key changes:
- Moved React hooks to UI package with global registration pattern for automatic resource hook attachment
- Updated documentation and exports to reflect new hook locations
- Preserved test coverage by relocating and expanding tests in the UI package
Reviewed Changes
Copilot reviewed 32 out of 34 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ui/src/index.ts | Updated exports to include migrated hooks instead of placeholder comments |
| packages/ui/src/hooks/usePolicy.ts | New implementation of usePolicy hook with runtime global resolution |
| packages/ui/src/hooks/useKernel.ts | Migrated useKernel implementation with kernel imports |
| packages/ui/src/hooks/resource-hooks.ts | New resource hook implementations with global attachment mechanism |
| packages/ui/src/hooks/tests/*.test.ts | Migrated and updated test files for all hook implementations |
| packages/ui/package.json | Added @wordpress/data peer dependency |
| packages/kernel/src/resource/define.ts | Removed inline hook implementations, added global hook attachment |
| packages/kernel/src/policy/index.ts | Removed usePolicy export |
| packages/kernel/src/index.ts | Removed useKernel and usePolicy exports |
| docs/* | Updated documentation to reference new hook locations |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
…gurations - Fixed import paths in useKernel.ts to use specific module paths for better tree-shaking - Removed unnecessary type assertions in useKernel.test.ts, using proper mock types - Added comment referencing shared global type definition in resource-hooks.ts - Fixed resource-hooks.test.ts mock to return undefined instead of null - Fixed TypeScript project references configuration: - Removed incompatible 'noEmit: true' from test configs (kernel & ui packages) - Added packages/ui/tsconfig.tests.json reference to root tsconfig.json - Added global types to UI test config include paths - All tests passing: 947 total (945 passed, 2 skipped) - All typecheck validations passing
Contributor
Author
|
@codex please review again and ensure your previous feedback is addressed |
Contributor
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 58 out of 62 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
packages/ui/src/hooks/useVisiblePrefetch.ts:1
- The
!non-null assertions are potentially unsafe here sincevalues[0]could be undefined iftokensis empty. Consider adding a guard condition or defaulting to'0px'whentokens.length === 0.
import { useEffect, useMemo, useRef } from 'react';
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
- Refactor complex type assertion chain in ensureDispatch() - Extract invokeMethod after type guard for cleaner code flow - Add explanatory comment for generic type casting responsibility - Document intentional primitive throw in test case for error normalization Addresses Copilot PR feedback #70 comments 5-6
Contributor
Author
|
@codex fixed your previous issues, review again please |
Contributor
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 58 out of 62 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
packages/ui/src/hooks/useAction.ts:1
- Throwing a primitive string instead of an Error object is an anti-pattern that makes error handling less predictable. Consider throwing
new Error('primitive failure')to maintain consistency with JavaScript error handling conventions.
import { useCallback, useMemo, useRef, useState } from 'react';
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
- Add module-level queue for resources created before UI bundle loads - Kernel side: Queue resources and expose __WP_KERNEL_UI_PROCESS_PENDING_RESOURCES__ - UI side: Process pending queue when resource-hooks.ts module loads - Add global type definition for process function - Fixes timing issue where resources defined before UI never get React hooks Addresses P1 review comment from PR #70
Contributor
Author
|
@codex any P1 or P2 issues? |
Findings
Recommendation
|
All Review Comments Resolved - Fixed import path issues (relative and absolute paths) - Improved type assertions with detailed explanatory comments - Enhanced JSDoc documentation for all new functionality - Fixed tsconfig test paths and configurations - Resolved resource hook timing issue with queuing mechanism - Fixed queue cancellation bug using cancellation token Comprehensive JSDoc Documentation - Added module-level documentation for resource-hooks.ts - Documented lazy binding and pending resource queue system - Enhanced type assertions with detailed explanations - Added comments for global hook registration - Documented WordPress data store integration patterns Expanded Test Coverage (89.73 percent branches) Added 50 plus new tests for uncovered branches: - resource-hooks.test.ts: SSR handling, pending queue processing - useAction.test.tsx: error paths, concurrency modes, cancellation - useVisiblePrefetch.test.tsx: SSR handling, margin parsing, observer edge cases - useHoverPrefetch.test.tsx: unmount cancellation, listener cleanup - useKernel.test.tsx: middleware handling and cleanup - usePolicy.test.tsx: promise rejection, runtime errors, success scenarios Bug Fixes - Fixed queue cancellation logic with queueCancelledRef - Implemented resource hook lazy binding with pending queue - Corrected non-null assertions in useVisiblePrefetch - Added type guard for WordPress data store dispatch - Improved error normalization in useAction hook Coverage Improvements - Overall: 97.00 percent statements, 89.73 percent branches, 98.01 percent functions - All UI hooks meet or exceed branch coverage thresholds - Comprehensive edge case testing, no regressions All tests passing (970 of 972), all type checks green. Refs: #70 Refs: #70
Contributor
Author
|
@codex, I believe all issues resolved? |
|
Summary
Testing
|
17 tasks
pipewrk
added a commit
that referenced
this pull request
Nov 8, 2025
…gurations - Fixed import paths in useKernel.ts to use specific module paths for better tree-shaking - Removed unnecessary type assertions in useKernel.test.ts, using proper mock types - Added comment referencing shared global type definition in resource-hooks.ts - Fixed resource-hooks.test.ts mock to return undefined instead of null - Fixed TypeScript project references configuration: - Removed incompatible 'noEmit: true' from test configs (kernel & ui packages) - Added packages/ui/tsconfig.tests.json reference to root tsconfig.json - Added global types to UI test config include paths - All tests passing: 947 total (945 passed, 2 skipped) - All typecheck validations passing
pipewrk
added a commit
that referenced
this pull request
Nov 8, 2025
- Refactor complex type assertion chain in ensureDispatch() - Extract invokeMethod after type guard for cleaner code flow - Add explanatory comment for generic type casting responsibility - Document intentional primitive throw in test case for error normalization Addresses Copilot PR feedback #70 comments 5-6
pipewrk
added a commit
that referenced
this pull request
Nov 8, 2025
- Add module-level queue for resources created before UI bundle loads - Kernel side: Queue resources and expose __WP_KERNEL_UI_PROCESS_PENDING_RESOURCES__ - UI side: Process pending queue when resource-hooks.ts module loads - Add global type definition for process function - Fixes timing issue where resources defined before UI never get React hooks Addresses P1 review comment from PR #70
pipewrk
added a commit
that referenced
this pull request
Nov 8, 2025
All Review Comments Resolved - Fixed import path issues (relative and absolute paths) - Improved type assertions with detailed explanatory comments - Enhanced JSDoc documentation for all new functionality - Fixed tsconfig test paths and configurations - Resolved resource hook timing issue with queuing mechanism - Fixed queue cancellation bug using cancellation token Comprehensive JSDoc Documentation - Added module-level documentation for resource-hooks.ts - Documented lazy binding and pending resource queue system - Enhanced type assertions with detailed explanations - Added comments for global hook registration - Documented WordPress data store integration patterns Expanded Test Coverage (89.73 percent branches) Added 50 plus new tests for uncovered branches: - resource-hooks.test.ts: SSR handling, pending queue processing - useAction.test.tsx: error paths, concurrency modes, cancellation - useVisiblePrefetch.test.tsx: SSR handling, margin parsing, observer edge cases - useHoverPrefetch.test.tsx: unmount cancellation, listener cleanup - useKernel.test.tsx: middleware handling and cleanup - usePolicy.test.tsx: promise rejection, runtime errors, success scenarios Bug Fixes - Fixed queue cancellation logic with queueCancelledRef - Implemented resource hook lazy binding with pending queue - Corrected non-null assertions in useVisiblePrefetch - Added type guard for WordPress data store dispatch - Improved error normalization in useAction hook Coverage Improvements - Overall: 97.00 percent statements, 89.73 percent branches, 98.01 percent functions - All UI hooks meet or exceed branch coverage thresholds - Comprehensive edge case testing, no regressions All tests passing (970 of 972), all type checks green. Refs: #70 Refs: #70
pipewrk
added a commit
that referenced
this pull request
Nov 8, 2025
Sprint 5: Bindings & Interactivity — migrate React hooks to UI
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Sprint 5: Bindings & Interactivity — migrate React hooks to UI.
Sprint: 5 (Bindings & Interactivity)
Scope: policy · resource · data · ui · docs
Links
Why
React-facing hooks lived in
@geekist/wp-kernel, tying the kernel package to React and WordPress data availability. Moving them into the UI package keeps the kernel transport-agnostic while providing a single UI integration surface.What
useKernel,usePolicy, and resourceuseGet/useListhelpers (plus tests) into@geekist/wp-kernel-ui, exposing globals so kernel resources attach hooks automatically once the UI bundle loads.How
Hook implementations now live beside UI components and rely on kernel runtime globals for registration.
defineResource()looks for a UI-provided global to attach hooks lazily, and documentation calls out the requirement to import@geekist/wp-kernel-uiwhen bootstrapping UI code. Legacy kernel tests were relocated and expanded under the UI package to preserve coverage.Testing
How reviewers test locally:
CI=1 pnpm lint --fixCI=1 pnpm typecheck && CI=1 pnpm typecheck:testsCI=1 pnpm testCI=1 pnpm buildExpected: All commands succeed without errors; tests remain green with full coverage.
Test Matrix (tick what’s covered)
Screenshots / Recordings (if applicable)
N/A
Breaking Changes
Affected Packages
@geekist/wp-kernel@geekist/wp-kernel-ui@geekist/wp-kernel-cli@geekist/wp-kernel-e2e-utilsRelease
Checklist
pnpm test, where relevant:pnpm e2e)pnpm lint)pnpm typecheck,pnpm typecheck:tests)no-release)https://chatgpt.com/codex/tasks/task_e_68e3cf789f148325ae4fbe555d4052be