-
Notifications
You must be signed in to change notification settings - Fork 402
feat(clerk-js,types): Signals fetchStatus #6549
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a2d6ba1
5065c9b
550d029
e2bd52f
5164243
d7f905a
d99ed16
37b024d
bf71255
2336512
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| '@clerk/clerk-js': minor | ||
| '@clerk/types': minor | ||
| --- | ||
|
|
||
| [Experimental] Signal `fetchStatus` support. |
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -4,20 +4,18 @@ import { computed, signal } from 'alien-signals'; | |||||||
|
|
||||||||
| import type { SignIn } from './resources/SignIn'; | ||||||||
|
|
||||||||
| export const signInSignal = signal<{ resource: SignIn | null }>({ resource: null }); | ||||||||
| export const signInResourceSignal = signal<{ resource: SignIn | null }>({ resource: null }); | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainRename to signInResourceSignal is clear; verify callers and mark as internal Good separation of responsibilities by renaming to signInResourceSignal. If this is not part of the public API, add an @internal JSDoc to comply with docs guidelines and prevent accidental external usage. Also, please verify there are no lingering imports/usages of the old signInSignal. Run this to ensure no leftover references exist: Apply this diff to mark the export as internal: +/** @internal */
export const signInResourceSignal = signal<{ resource: SignIn | null }>({ resource: null });🏁 Script executed: #!/bin/bash
set -euo pipefail
# Find any remaining references to the old name
rg -n --heading '\bsignInSignal\b' -g '!**/node_modules/**'Length of output: 365 Update leftover We’ve got three remaining
Please make these changes to fully remove the old 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||
| export const signInErrorSignal = signal<{ error: unknown }>({ error: null }); | ||||||||
| export const signInFetchSignal = signal<{ status: 'idle' | 'fetching' }>({ status: 'idle' }); | ||||||||
|
|
||||||||
| export const signInComputedSignal = computed(() => { | ||||||||
| const signIn = signInSignal().resource; | ||||||||
| const signIn = signInResourceSignal().resource; | ||||||||
| const error = signInErrorSignal().error; | ||||||||
| const fetchStatus = signInFetchSignal().status; | ||||||||
|
|
||||||||
| const errors = errorsToParsedErrors(error); | ||||||||
|
|
||||||||
| if (!signIn) { | ||||||||
| return { errors, signIn: null }; | ||||||||
| } | ||||||||
|
|
||||||||
| return { errors, signIn: signIn.__internal_future }; | ||||||||
| return { errors, fetchStatus, signIn: signIn ? signIn.__internal_future : null }; | ||||||||
| }); | ||||||||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
|
|
||||||||
| /** | ||||||||
|
|
@@ -42,6 +40,10 @@ function errorsToParsedErrors(error: unknown): Errors { | |||||||
| global: [], | ||||||||
| }; | ||||||||
|
|
||||||||
| if (!error) { | ||||||||
| return parsedErrors; | ||||||||
| } | ||||||||
|
|
||||||||
| if (!isClerkAPIResponseError(error)) { | ||||||||
| parsedErrors.raw.push(error); | ||||||||
| parsedErrors.global.push(error); | ||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| import { afterEach, describe, expect, it, vi } from 'vitest'; | ||
|
|
||
| import { eventBus } from '../../core/events'; | ||
| import { runAsyncResourceTask } from '../runAsyncResourceTask'; | ||
|
|
||
| describe('runAsyncTask', () => { | ||
| afterEach(() => { | ||
| vi.restoreAllMocks(); | ||
| }); | ||
|
|
||
| const resource = {} as any; // runAsyncTask doesn't depend on resource being a BaseResource | ||
|
|
||
| it('emits fetching/idle and returns result on success', async () => { | ||
| const emitSpy = vi.spyOn(eventBus, 'emit'); | ||
| const task = vi.fn().mockResolvedValue('ok'); | ||
|
|
||
| const { result, error } = await runAsyncResourceTask(resource, task); | ||
|
|
||
| expect(task).toHaveBeenCalledTimes(1); | ||
| expect(result).toBe('ok'); | ||
| expect(error).toBeNull(); | ||
|
|
||
| expect(emitSpy).toHaveBeenNthCalledWith(1, 'resource:error', { | ||
| resource, | ||
| error: null, | ||
| }); | ||
| expect(emitSpy).toHaveBeenNthCalledWith(2, 'resource:fetch', { | ||
| resource, | ||
| status: 'fetching', | ||
| }); | ||
| expect(emitSpy).toHaveBeenNthCalledWith(3, 'resource:fetch', { | ||
| resource, | ||
| status: 'idle', | ||
| }); | ||
| }); | ||
|
|
||
| it('emits error and returns error on failure', async () => { | ||
| const emitSpy = vi.spyOn(eventBus, 'emit'); | ||
| const thrown = new Error('fail'); | ||
| const task = vi.fn().mockRejectedValue(thrown); | ||
|
|
||
| const { result, error } = await runAsyncResourceTask(resource, task); | ||
|
|
||
| expect(task).toHaveBeenCalledTimes(1); | ||
| expect(result).toBeUndefined(); | ||
| expect(error).toBe(thrown); | ||
|
|
||
| expect(emitSpy).toHaveBeenNthCalledWith(1, 'resource:error', { | ||
| resource, | ||
| error: null, | ||
| }); | ||
| expect(emitSpy).toHaveBeenNthCalledWith(2, 'resource:fetch', { | ||
| resource, | ||
| status: 'fetching', | ||
| }); | ||
| expect(emitSpy).toHaveBeenNthCalledWith(3, 'resource:error', { | ||
| resource, | ||
| error: thrown, | ||
| }); | ||
| expect(emitSpy).toHaveBeenNthCalledWith(4, 'resource:fetch', { | ||
| resource, | ||
| status: 'idle', | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid sending identifier: null; include the field only when defined
Using identifier || previousIdentifier can yield null, which will serialize as identifier: null. Prefer nullish coalescing and omit the key when undefined.
📝 Committable suggestion
🤖 Prompt for AI Agents