feat: redesign durably-react hooks for React 19#179
Merged
Conversation
Remove loading/error state from useRunActions (callers use useTransition), add isTerminal/isActive to hooks and ClientRun, make createJobHooks forward options transparently. Bump to 0.15.0. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
Contributor
📝 WalkthroughWalkthroughdurably-react ライブラリを React 19 向けに再設計し、ホック群に Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 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. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
ClientRun now includes isTerminal/isActive derived fields. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix delete → deleteRun naming in index.md hook table - Handle getRun returning null in showDetails (both dashboards) - Show detailError outside modal when selectedRun is null Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Version bumps will be done in a separate release PR. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
packages/durably-react/tests/client/create-job-hooks.test.tsx (1)
83-100: このテストは「オプション転送」を実証できていません。Line 83-100 は
triggerの存在確認のみで、followLatest/autoResumeが実際に下位useJobに渡ったかは検証できません。転送バグがあっても通るため、意図に対して弱いです。✅ 期待挙動を直接検証する差分例
it('forwards optional useJob options to the underlying hook', async () => { const fetchMock = vi.fn().mockResolvedValue({ ok: true, json: () => Promise.resolve({ runId: 'csv-run-id' }), }) globalThis.fetch = fetchMock const hooks = createJobHooks<typeof importCsvJob>({ api: '/api/durably', jobName: 'import-csv', }) const { result } = renderHook(() => hooks.useJob({ followLatest: false, autoResume: false }), ) - expect(result.current.trigger).toBeTypeOf('function') + // autoResume が無効なら mount 時に runs 取得しない + expect(fetchMock).not.toHaveBeenCalled() + + await result.current.trigger({ filename: 'data.csv' }) + expect(fetchMock).toHaveBeenCalledWith( + '/api/durably/trigger', + expect.objectContaining({ method: 'POST' }), + ) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/durably-react/tests/client/create-job-hooks.test.tsx` around lines 83 - 100, The test only checks trigger exists but doesn't assert the optional flags are forwarded; update the test that uses createJobHooks and hooks.useJob to actually call result.current.trigger(...) (or otherwise execute the underlying request) and then assert the mocked fetch (fetchMock) received a request whose payload or query includes the followLatest and autoResume values you passed to hooks.useJob({ followLatest: false, autoResume: false }); specifically, after renderHook(() => hooks.useJob(...)) invoke the hook action (trigger) and inspect fetchMock.mock.calls to verify the request body or URL contains followLatest and autoResume, using the symbols createJobHooks, hooks.useJob, trigger, followLatest, and autoResume to locate the relevant code.packages/durably-react/src/client/use-run-actions.ts (1)
68-100: ドキュメント内の使用例が新しいパターンを適切に示しています。
useTransitionを使用したペンディング状態の管理と.catch()によるエラーハンドリングが示されており、React 19 のベストプラクティスに沿っています。ただし、例の中で
.catch(() => {})は空のハンドラーになっています。本番コードでは適切なエラーハンドリング（ユーザーへの通知やログ記録）が必要です。ドキュメント例としては簡潔さのために許容されますが、コメントで本番では適切なエラー処理が必要であることを示唆することを検討してください。💡 より明確な例への修正案
* <button * onClick={() => -* startTransition(() => retrigger(runId).catch(() => {})) +* startTransition(() => retrigger(runId).catch(console.error)) * }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/durably-react/src/client/use-run-actions.ts` around lines 68 - 100, Update the example in the RunActions component that uses useRunActions, retrigger, cancel and useTransition to note that the current .catch(() => {}) is only for brevity; modify the JSX example near RunActions to replace the empty catch with a brief inline comment (or a short example call) indicating that production code should perform proper error handling (e.g., logging or user notification) when calling retrigger(runId) and cancel(runId), and ensure references to isPending and startTransition remain unchanged so the example still demonstrates transition handling correctly.website/guide/error-handling.md (1)
64-72: エラーハンドリングパターンの一貫性について検討してください。Retriggerボタン（Lines 49-54）では
try/catchを使用していますが、Cancelボタンではvoid ... .catch()パターンを使用しています。両方とも有効なアプローチですが、同じドキュメント内で異なるパターンを使用すると読者が混乱する可能性があります。教育目的で両方のパターンを意図的に示している場合は、そのことを明示的にコメントで説明するか、どちらか一方に統一することを検討してください。
💡 一貫性のある例への修正案
if (status === 'leased') { return ( <button - onClick={() => { - void cancel(runId).catch((e) => console.error('Cancel failed:', e)) - }} + onClick={async () => { + try { + await cancel(runId) + } catch (e) { + console.error('Cancel failed:', e) + } + }} > Cancel </button> ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/guide/error-handling.md` around lines 64 - 72, ドキュメント内のエラーハンドリングが一貫しておらず読者が混乱するので、Cancelボタンの onClick（現在の void cancel(runId).catch(...)）を Retrigger ボタンで使っている try/catch パターンに統一するか、両方を並べて示す意図があることを明示するコメントを追加してください；具体的には cancel(runId) 呼び出しを async ハンドラ内で try { await cancel(runId) } catch (e) { console.error('Cancel failed:', e) } の形式に変更するか、両方を並列で示すセクションに「意図的に異なるパターンを比較している」旨の短い説明を加えてください。packages/durably-react/tests/types.test.ts (1)
125-153:createJobHooks.useLogsの型もここで固定しておきたいです。
create-job-hooks.tsではuseLogsもOmit<UseJobLogsClientOptions, 'api' | 'runId'>に変わっているので、このスイートがuseJob/useRunだけを固定していると残り 1 本の透過ラッパーだけ回帰を見逃します。Hooks['useLogs']の引数型も同じ形で追加しておくと安心です。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/durably-react/tests/types.test.ts` around lines 125 - 153, The test suite pins types for useJob and useRun but misses useLogs; update the tests to also assert Hooks['useLogs'] parameter type matches Omit<UseJobLogsClientOptions, 'api' | 'runId'> | undefined so changes in createJobHooks propagate to tests. Locate the test block for createJobHooks (where JobHooks is aliased to Hooks) and add an expectTypeOf<Hooks['useLogs']>().parameter(0).toEqualTypeOf< Omit< import('../src/client/use-job-logs').UseJobLogsClientOptions, 'api' | 'runId' > | undefined >() referencing the Hooks type and UseJobLogsClientOptions to lock the wrapper’s argument shape.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/fullstack-react-router/app/routes/_index/dashboard.tsx`:
- Around line 96-110: Clear previous detail state when starting and on failure,
and ensure the error is surfaced independently of the selectedRun modal. In
showDetails(), before calling getRun/getSteps call setDetailError(null),
setDetailLoading(true), setSteps([]) and setSelectedRun(null) so stale
steps/selectedRun aren’t shown if fetch fails; in the catch block keep calling
setDetailError(e instanceof Error ? e.message : String(e)) and also setSteps([])
and setSelectedRun(null) to fully reset detail state on error. Finally, update
UI usage so it reads the standalone detailError state (not a field under
selectedRun) so errors are visible even when the modal/selectedRun is not set;
apply the same fixes to the other occurrence referenced at lines ~317-319 (same
showDetails usage).
In `@examples/fullstack-vercel-turso/app/routes/_index/dashboard.tsx`:
- Around line 109-123: The showDetails error path can leave stale detail state
(previous steps/selectedRun) visible if getRun/getSteps fails; fix by resetting
detail state at the start of the load: call setSteps([]), setSelectedRun(null)
and setDetailError(null) before setDetailLoading(true) (so the UI clears
previous data and will render errors even when selectedRun is null), then
perform getRun/getSteps in the try and on catch setDetailError(e instanceof
Error ? e.message : String(e)) and ensure setSteps([]) / setSelectedRun(null)
remain if the fetch fails.
In `@packages/durably-react/src/client/create-job-hooks.ts`:
- Around line 95-110: The factory currently spreads fixed values (api, jobName,
runId) before caller options which lets caller options accidentally overwrite
those fixed values; update the three wrappers so caller-provided options are
spread first and the fixed values are spread last. Specifically, in the useJob
wrapper (useJob: (jobOptions) => ...), spread jobOptions before adding api and
jobName; in useRun (useRun: (runId, runOptions) => ...) spread runOptions before
adding api and runId; and in useLogs (useLogs: (runId, logsOptions) => ...)
spread logsOptions before adding api and runId so the wrapper’s
api/jobName/runId cannot be overridden by caller options.
In `@packages/durably-react/tests/client/use-run-actions.test.tsx`:
- Around line 244-273: The test's unhandledrejection listener can miss
rejections because retrigger('run-123') involves multiple awaits; capture the
Promise returned by retrigger (from the onClick call in Harness) outside the act
block, await it (with a try/catch if needed) before removing the listener, and
move globalThis.removeEventListener('unhandledrejection', onUnhandled) into a
finally block so the listener is only removed after the retrigger promise has
fully settled; refer to the retrigger call in Harness, the onUnhandled listener
and the unhandled array when making this change.
In `@website/api/http-handler.md`:
- Line 100: The documentation's ClientRun field exclusion list omits leaseOwner,
which can mislead readers about what /runs and /run return; update the
descriptive list (the sentence that lists internal fields for ClientRun) to
include leaseOwner alongside leaseExpiresAt, idempotencyKey, concurrencyKey,
leaseGeneration, updatedAt, etc., and ensure the note about using toClientRun()
mentions that leaseOwner is also stripped by that projection; apply the same
amendment to the second occurrence referenced (the other sentence near line
106).
---
Nitpick comments:
In `@packages/durably-react/src/client/use-run-actions.ts`:
- Around line 68-100: Update the example in the RunActions component that uses
useRunActions, retrigger, cancel and useTransition to note that the current
.catch(() => {}) is only for brevity; modify the JSX example near RunActions to
replace the empty catch with a brief inline comment (or a short example call)
indicating that production code should perform proper error handling (e.g.,
logging or user notification) when calling retrigger(runId) and cancel(runId),
and ensure references to isPending and startTransition remain unchanged so the
example still demonstrates transition handling correctly.
In `@packages/durably-react/tests/client/create-job-hooks.test.tsx`:
- Around line 83-100: The test only checks trigger exists but doesn't assert the
optional flags are forwarded; update the test that uses createJobHooks and
hooks.useJob to actually call result.current.trigger(...) (or otherwise execute
the underlying request) and then assert the mocked fetch (fetchMock) received a
request whose payload or query includes the followLatest and autoResume values
you passed to hooks.useJob({ followLatest: false, autoResume: false });
specifically, after renderHook(() => hooks.useJob(...)) invoke the hook action
(trigger) and inspect fetchMock.mock.calls to verify the request body or URL
contains followLatest and autoResume, using the symbols createJobHooks,
hooks.useJob, trigger, followLatest, and autoResume to locate the relevant code.
In `@packages/durably-react/tests/types.test.ts`:
- Around line 125-153: The test suite pins types for useJob and useRun but
misses useLogs; update the tests to also assert Hooks['useLogs'] parameter type
matches Omit<UseJobLogsClientOptions, 'api' | 'runId'> | undefined so changes in
createJobHooks propagate to tests. Locate the test block for createJobHooks
(where JobHooks is aliased to Hooks) and add an
expectTypeOf<Hooks['useLogs']>().parameter(0).toEqualTypeOf< Omit<
import('../src/client/use-job-logs').UseJobLogsClientOptions, 'api' | 'runId' >
| undefined >() referencing the Hooks type and UseJobLogsClientOptions to lock
the wrapper’s argument shape.
In `@website/guide/error-handling.md`:
- Around line 64-72: ドキュメント内のエラーハンドリングが一貫しておらず読者が混乱するので、Cancelボタンの onClick（現在の
void cancel(runId).catch(...)）を Retrigger ボタンで使っている try/catch
パターンに統一するか、両方を並べて示す意図があることを明示するコメントを追加してください；具体的には cancel(runId) 呼び出しを async
ハンドラ内で try { await cancel(runId) } catch (e) { console.error('Cancel failed:',
e) } の形式に変更するか、両方を並列で示すセクションに「意図的に異なるパターンを比較している」旨の短い説明を加えてください。
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 63b1f8fa-93cf-44b7-801c-0bebd9b01b61
📒 Files selected for processing (32)
examples/fullstack-react-router/app/routes/_index/dashboard.tsxexamples/fullstack-vercel-turso/app/routes/_index/dashboard.tsxpackages/durably-react/docs/llms.mdpackages/durably-react/package.jsonpackages/durably-react/src/client/create-job-hooks.tspackages/durably-react/src/client/use-job-run.tspackages/durably-react/src/client/use-job.tspackages/durably-react/src/client/use-run-actions.tspackages/durably-react/src/hooks/use-job-run.tspackages/durably-react/src/hooks/use-job.tspackages/durably-react/src/types.tspackages/durably-react/tests/browser/use-job-run.test.tsxpackages/durably-react/tests/browser/use-job.test.tsxpackages/durably-react/tests/client/create-job-hooks.test.tsxpackages/durably-react/tests/client/use-job-run.test.tsxpackages/durably-react/tests/client/use-job.test.tsxpackages/durably-react/tests/client/use-run-actions.test.tsxpackages/durably-react/tests/client/use-runs.test.tsxpackages/durably-react/tests/types.test.tspackages/durably/docs/llms.mdpackages/durably/package.jsonpackages/durably/src/storage.tspackages/durably/tests/shared/server.shared.tswebsite/api/durably-react/fullstack.mdwebsite/api/durably-react/index.mdwebsite/api/durably-react/spa.mdwebsite/api/durably-react/types.mdwebsite/api/http-handler.mdwebsite/api/index.mdwebsite/guide/error-handling.mdwebsite/guide/fullstack-mode.mdwebsite/public/llms.txt
…ntRun exclusion list - Clear selectedRun and steps at start of showDetails to prevent stale data on error - Add leaseOwner to ClientRun excluded fields list in http-handler.md Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
coji
force-pushed
the
feat/react19-hooks-redesign
branch
from
cd1800b to
d6ae06d
Compare
- JSDoc example: replace empty .catch(() => {}) with .catch(console.error)
- createJobHooks test: assert autoResume:false prevents fetch on mount
- Type test: add useLogs wrapper parameter assertion
- error-handling guide: unify cancel to try/catch pattern
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
coji
force-pushed
the
feat/react19-hooks-redesign
branch
from
870a407 to
abee861
Compare
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
1 participant
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
Closes #160
useRunActionsstate:isLoadinganderrorremoved from return type — callers use React 19useTransitionfor per-button pending UI and try/catch for errorscreateJobHooks: options forwarded viaOmit<Options, 'api' | ...>so new options auto-propagateisTerminal/isActiveon hooks andClientRun: derived status booleans replace manual status enumerationBreaking Changes
useRunActions()returnsisLoading,erroruseTransition+ local staterun.status === 'completed' || ...run.isTerminalcreateJobHooks().useRun(id, { onComplete })onlyUseJobRunClientOptionsexceptapi/runIdTest plan
pnpm validatepasses (format, lint, typecheck, tests — 28/28)UseRunActionsClientResultkeys,isTerminal/isActiveon hooks)useTransition+Set<string>busy trackingfind-stale.shdoc check cleanwebsite/public/llms.txtregenerated🤖 Generated with Claude Code