feat(clerk-js,ui,shared): route <ConfigureSSO /> through org-scoped enterprise_connections#8671
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 0d8f86e The changes in this PR will be included in the next version bump. This PR includes changesets to release 20 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
| '@clerk/shared': minor | ||
| --- | ||
|
|
||
| `<ConfigureSSO />` now calls the org-scoped enterprise connections endpoints via `organization.*EnterpriseConnection*` methods. Previously, the wizard called `user.*EnterpriseConnection*` against the `/me/*` paths. |
There was a problem hiding this comment.
| `<ConfigureSSO />` now calls the org-scoped enterprise connections endpoints via `organization.*EnterpriseConnection*` methods. Previously, the wizard called `user.*EnterpriseConnection*` against the `/me/*` paths. | |
| Internal `<ConfigureSSO />` refactor to call new org-scoped enterprise connections FAPI endpoints, replacing the `/me/` deprecated scope |
Changing the copy a bit here to denote that this is an internal change, not important for external consumers
| --- | ||
| '@clerk/clerk-js': minor | ||
| '@clerk/shared': minor | ||
| --- | ||
|
|
||
| Add organization-scoped enterprise connection methods on the `Organization` resource: `getEnterpriseConnections`, `createEnterpriseConnection`, `updateEnterpriseConnection`, `deleteEnterpriseConnection`, `createEnterpriseConnectionTestRun`, and `getEnterpriseConnectionTestRuns`. These hit `/v1/organizations/{org_id}/enterprise_connections/*` and share the same flattened SAML/OIDC request body shape as the existing `User.*` equivalents. | ||
|
|
||
| Renames the parameter types from `Me*` to `Organization*`: | ||
| - `CreateOrganizationEnterpriseConnectionParams` | ||
| - `UpdateOrganizationEnterpriseConnectionParams` | ||
| - `OrganizationEnterpriseConnectionProvider` | ||
| - `OrganizationEnterpriseConnectionSamlInput` | ||
| - `OrganizationEnterpriseConnectionOidcInput` | ||
|
|
||
| The previous `Me*` names remain available as `@deprecated` aliases for backwards compatibility. |
There was a problem hiding this comment.
I think we can keep all on the same changeset, as those endpoints aren't public on the frontend API as well, it's not like developers were relying on the clerk-js resources for it
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (19)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (17)
📝 WalkthroughWalkthroughThis PR migrates enterprise-connection flows from user-scoped (/me/) APIs to organization-scoped endpoints. It adds Organization-scoped types (with deprecated Me* aliases), a shared toEnterpriseConnectionBody serializer, six new Organization resource methods (list/create/update/delete enterprise connections; create/list test runs), updates User resource methods to use the shared serializer and Organization param types, introduces stable cache keys and two React hooks for organization enterprise connections and test runs (with polling and CRUD wrappers), and updates ConfigureSSO UI/context/tests to use the organization-scoped hooks and APIs. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ui/src/components/ConfigureSSO/__tests__/ConfigureSSO.test.tsx (1)
54-69:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClarify/remove the enterprise-connection mock in the personal-workspace test.
In the “in a personal workspace” setup,withUser({ email_addresses: ... })leavessession.lastActiveOrganizationIdnull (noorganization_memberships), sofixtures.clerk.organizationisnulland the enterprise-connections hook won’t callorganization.getEnterpriseConnections(it’s gated byBoolean(organization)). As a result,fixtures.clerk.organization?.getEnterpriseConnections.mockResolvedValue([])is effectively a no-op—either remove it or assertfixtures.clerk.organizationis null for intent.🤖 Prompt for 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. In `@packages/ui/src/components/ConfigureSSO/__tests__/ConfigureSSO.test.tsx` around lines 54 - 69, The enterprise-connections mock in the "in a personal workspace" test is a no-op because withUser(...) leaves session.lastActiveOrganizationId null so fixtures.clerk.organization is null; either remove the ineffective mock (fixtures.clerk.organization?.getEnterpriseConnections.mockResolvedValue([])) or make the test explicit by asserting fixtures.clerk.organization is null to signal intent; update the ConfigureSSO test block (references: withUser, withEnterpriseSso, fixtures.clerk.organization?.getEnterpriseConnections) accordingly so the test is clear and not misleading.
🧹 Nitpick comments (6)
packages/clerk-js/src/core/resources/__tests__/Organization.test.ts (2)
105-170: ⚡ Quick winReplace
@ts-ignorewith properly typed mocks.Lines 105, 143, 169: Multiple
@ts-ignoredirectives bypass type checking when mockingBaseResource._fetch. Use Vitest's type-safe mocking patterns instead:// Create a properly typed mock helper const mockFetch = <T>(response: T) => { return vi.spyOn(BaseResource as any, '_fetch').mockResolvedValue({ response }); }; // Then use it in tests const mock = mockFetch(enterpriseConnectionsJSON);This eliminates type safety bypasses while maintaining test functionality.
🤖 Prompt for 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. In `@packages/clerk-js/src/core/resources/__tests__/Organization.test.ts` around lines 105 - 170, Tests currently use multiple `@ts-ignore` to mock BaseResource._fetch; create a typed mock helper that uses vi.spyOn(BaseResource, '_fetch') with mockResolvedValue to return { response } instead of suppressing types. Replace each occurrence of the `@ts-ignore` + vi.fn() pattern in the Organization tests (references: BaseResource._fetch, createOrganization, createEnterpriseConnection, getEnterpriseConnections) with calls to this helper (e.g., mockFetch(enterpriseConnectionsJSON) / mockFetch(enterpriseConnectionJSON)) and assert against BaseResource._fetch.mock.calls as before; ensure the spy is properly typed and restored/reset between tests.
196-222: ⚡ Quick winReplace
@ts-ignorewith properly typed mocks.Same issue as the earlier test cases: use properly typed mock utilities instead of
@ts-ignoredirectives to accessBaseResource._fetchand its mock properties.Also applies to: 231-246, 249-263, 279-302
🤖 Prompt for 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. In `@packages/clerk-js/src/core/resources/__tests__/Organization.test.ts` around lines 196 - 222, Remove the `@ts-ignore` usages and replace them with a properly typed mock for BaseResource._fetch: create a typed spy like `const fetchMock = vi.spyOn(BaseResource, '_fetch') as MockedFunction<typeof BaseResource._fetch>` (import MockedFunction from 'vitest'), then call `fetchMock.mockResolvedValue({ response: enterpriseConnectionJSON })` and assert against `fetchMock.mock.calls[...]` (instead of using BaseResource._fetch with `@ts-ignore`); apply the same pattern for the other test blocks that touch BaseResource._fetch (lines around 231-246, 249-263, 279-302) and keep references to createOrganization and organization.updateEnterpriseConnection in the tests.packages/clerk-js/src/core/resources/Organization.ts (1)
166-265: 💤 Low valueConsider stronger typing for request body parameters.
Lines 195 and 210 cast the result of
toEnterpriseConnectionBodytoas any. While functional, this bypasses type checking. Consider one of these approaches:
- Type the
bodyparameter inBaseResource._fetchmore precisely- Create a branded type for enterprise connection request bodies
- Use a type assertion to a more specific interface
This is consistent with existing patterns in the codebase but could be improved for better type safety.
🤖 Prompt for 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. In `@packages/clerk-js/src/core/resources/Organization.ts` around lines 166 - 265, The createEnterpriseConnection and updateEnterpriseConnection methods currently pass body: toEnterpriseConnectionBody(... ) as any to BaseResource._fetch which defeats type checking; change this by introducing a specific type (e.g., EnterpriseConnectionRequestBody or a branded type) that matches the output of toEnterpriseConnectionBody and update BaseResource._fetch generic/type signature (or assert to that specific interface) so the body parameter is strongly typed instead of using any; update calls in createEnterpriseConnection and updateEnterpriseConnection to use that specific type (referencing toEnterpriseConnectionBody, BaseResource._fetch, createEnterpriseConnection, updateEnterpriseConnection).packages/shared/src/react/hooks/useOrganizationEnterpriseConnections.shared.ts (1)
9-12: 💤 Low valueAdd an explicit return type to this exported hook.
This is an exported function returning the
createCacheKeysresult implicitly. Annotating the return type keeps the public/internal surface explicit and consistent with the.tsxhooks in this cohort.♻️ Proposed annotation
export function useOrganizationEnterpriseConnectionsCacheKeys(params: { organizationId: string | null; withOrganizationAccountLinking?: boolean; -}) { +}): ReturnType<typeof createCacheKeys> {Based on learnings: enforce explicit return type annotations for exported functions and public APIs.
🤖 Prompt for 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. In `@packages/shared/src/react/hooks/useOrganizationEnterpriseConnections.shared.ts` around lines 9 - 12, The exported hook useOrganizationEnterpriseConnectionsCacheKeys currently returns the result of createCacheKeys implicitly; add an explicit return type annotation to the function signature to make the public API explicit and consistent with other hooks. Update the signature of useOrganizationEnterpriseConnectionsCacheKeys to include the correct return type (the type returned by createCacheKeys) so callers and consumers see the exact shape; reference createCacheKeys to infer the precise type and import or export any necessary types if not already available.packages/shared/src/react/hooks/useOrganizationEnterpriseConnectionTestRuns.shared.ts (1)
10-14: 💤 Low valueAdd an explicit return type to this exported hook.
Same as the connections cache-key hook: annotate the return type for consistency with the
.tsxhooks.♻️ Proposed annotation
export function useOrganizationEnterpriseConnectionTestRunsCacheKeys(params: { organizationId: string | null; enterpriseConnectionId: string | null; args: GetEnterpriseConnectionTestRunsParams; -}) { +}): ReturnType<typeof createCacheKeys> {Based on learnings: enforce explicit return type annotations for exported functions and public APIs.
🤖 Prompt for 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. In `@packages/shared/src/react/hooks/useOrganizationEnterpriseConnectionTestRuns.shared.ts` around lines 10 - 14, The exported hook useOrganizationEnterpriseConnectionTestRunsCacheKeys lacks an explicit return type; update its declaration to include the same annotated return type style used by the connections cache-key hook so the function signature is explicit and consistent with other .tsx hooks—locate the function useOrganizationEnterpriseConnectionTestRunsCacheKeys (which accepts organizationId, enterpriseConnectionId, args: GetEnterpriseConnectionTestRunsParams) and add the appropriate return type annotation to the exported function signature.packages/shared/src/react/hooks/useOrganizationEnterpriseConnections.tsx (1)
72-75: ⚡ Quick winRevalidate invalidation key scope
createCacheKeyssetsstableKeytostablePrefix(the first element ofqueryKey: [stablePrefix, authenticated, tracked, untracked]), so invalidating withqueryKey: [stableKey]inuseOrganizationEnterpriseConnectionstargets the same queryKey prefix the hook uses and isn’t likely to “silently fail”. The test-runs hook uses the more specificinvalidationKey([stablePrefix, authenticated, tracked]) for tighter invalidation scope; switching this hook toinvalidationKeywould be an optional consistency/tightness improvement.🤖 Prompt for 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. In `@packages/shared/src/react/hooks/useOrganizationEnterpriseConnections.tsx` around lines 72 - 75, The revalidate callback in useOrganizationEnterpriseConnections currently invalidates with queryClient.invalidateQueries({ queryKey: [stableKey] }) which uses the broader stableKey prefix from createCacheKeys; update revalidate to invalidate using the more specific invalidationKey (the array [stablePrefix, authenticated, tracked]) instead so it matches the hook's query scope—locate the revalidate definition in useOrganizationEnterpriseConnections and replace the queryKey argument to use the invalidationKey variable (or construct [stablePrefix, authenticated, tracked]) when calling queryClient.invalidateQueries to tighten the invalidation scope.
🤖 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 @.changeset/wide-items-flow.md:
- Line 7: Update the changelog sentence about the `<ConfigureSSO />` refactor to
state that org-scoped enterprise connection FAPI endpoints are being added
alongside — not replacing — the existing user-scoped `/v1/me/*` methods;
specifically rephrase to mention that organization-scoped methods (Organization
resource methods) are added in addition to the existing user-scoped `/me/`
endpoints so it no longer implies removal or deprecation of the `/me/` scope.
In `@packages/shared/src/types/organization.ts`:
- Around line 79-94: Add JSDoc comments for the six public OrganizationResource
methods: getEnterpriseConnections, createEnterpriseConnection,
updateEnterpriseConnection, deleteEnterpriseConnection,
createEnterpriseConnectionTestRun, and getEnterpriseConnectionTestRuns; for each
comment include a short description, `@param` annotations for each parameter
type/name (e.g., params, enterpriseConnectionId), an `@returns` describing the
Promise-wrapped return type (e.g., Promise<EnterpriseConnectionResource> or
ClerkPaginatedResponse<EnterpriseConnectionTestRunResource>), and a brief usage
example or note about optional params where applicable so the public API is
fully documented per guidelines.
---
Outside diff comments:
In `@packages/ui/src/components/ConfigureSSO/__tests__/ConfigureSSO.test.tsx`:
- Around line 54-69: The enterprise-connections mock in the "in a personal
workspace" test is a no-op because withUser(...) leaves
session.lastActiveOrganizationId null so fixtures.clerk.organization is null;
either remove the ineffective mock
(fixtures.clerk.organization?.getEnterpriseConnections.mockResolvedValue([])) or
make the test explicit by asserting fixtures.clerk.organization is null to
signal intent; update the ConfigureSSO test block (references: withUser,
withEnterpriseSso, fixtures.clerk.organization?.getEnterpriseConnections)
accordingly so the test is clear and not misleading.
---
Nitpick comments:
In `@packages/clerk-js/src/core/resources/__tests__/Organization.test.ts`:
- Around line 105-170: Tests currently use multiple `@ts-ignore` to mock
BaseResource._fetch; create a typed mock helper that uses vi.spyOn(BaseResource,
'_fetch') with mockResolvedValue to return { response } instead of suppressing
types. Replace each occurrence of the `@ts-ignore` + vi.fn() pattern in the
Organization tests (references: BaseResource._fetch, createOrganization,
createEnterpriseConnection, getEnterpriseConnections) with calls to this helper
(e.g., mockFetch(enterpriseConnectionsJSON) /
mockFetch(enterpriseConnectionJSON)) and assert against
BaseResource._fetch.mock.calls as before; ensure the spy is properly typed and
restored/reset between tests.
- Around line 196-222: Remove the `@ts-ignore` usages and replace them with a
properly typed mock for BaseResource._fetch: create a typed spy like `const
fetchMock = vi.spyOn(BaseResource, '_fetch') as MockedFunction<typeof
BaseResource._fetch>` (import MockedFunction from 'vitest'), then call
`fetchMock.mockResolvedValue({ response: enterpriseConnectionJSON })` and assert
against `fetchMock.mock.calls[...]` (instead of using BaseResource._fetch with
`@ts-ignore`); apply the same pattern for the other test blocks that touch
BaseResource._fetch (lines around 231-246, 249-263, 279-302) and keep references
to createOrganization and organization.updateEnterpriseConnection in the tests.
In `@packages/clerk-js/src/core/resources/Organization.ts`:
- Around line 166-265: The createEnterpriseConnection and
updateEnterpriseConnection methods currently pass body:
toEnterpriseConnectionBody(... ) as any to BaseResource._fetch which defeats
type checking; change this by introducing a specific type (e.g.,
EnterpriseConnectionRequestBody or a branded type) that matches the output of
toEnterpriseConnectionBody and update BaseResource._fetch generic/type signature
(or assert to that specific interface) so the body parameter is strongly typed
instead of using any; update calls in createEnterpriseConnection and
updateEnterpriseConnection to use that specific type (referencing
toEnterpriseConnectionBody, BaseResource._fetch, createEnterpriseConnection,
updateEnterpriseConnection).
In
`@packages/shared/src/react/hooks/useOrganizationEnterpriseConnections.shared.ts`:
- Around line 9-12: The exported hook
useOrganizationEnterpriseConnectionsCacheKeys currently returns the result of
createCacheKeys implicitly; add an explicit return type annotation to the
function signature to make the public API explicit and consistent with other
hooks. Update the signature of useOrganizationEnterpriseConnectionsCacheKeys to
include the correct return type (the type returned by createCacheKeys) so
callers and consumers see the exact shape; reference createCacheKeys to infer
the precise type and import or export any necessary types if not already
available.
In `@packages/shared/src/react/hooks/useOrganizationEnterpriseConnections.tsx`:
- Around line 72-75: The revalidate callback in
useOrganizationEnterpriseConnections currently invalidates with
queryClient.invalidateQueries({ queryKey: [stableKey] }) which uses the broader
stableKey prefix from createCacheKeys; update revalidate to invalidate using the
more specific invalidationKey (the array [stablePrefix, authenticated, tracked])
instead so it matches the hook's query scope—locate the revalidate definition in
useOrganizationEnterpriseConnections and replace the queryKey argument to use
the invalidationKey variable (or construct [stablePrefix, authenticated,
tracked]) when calling queryClient.invalidateQueries to tighten the invalidation
scope.
In
`@packages/shared/src/react/hooks/useOrganizationEnterpriseConnectionTestRuns.shared.ts`:
- Around line 10-14: The exported hook
useOrganizationEnterpriseConnectionTestRunsCacheKeys lacks an explicit return
type; update its declaration to include the same annotated return type style
used by the connections cache-key hook so the function signature is explicit and
consistent with other .tsx hooks—locate the function
useOrganizationEnterpriseConnectionTestRunsCacheKeys (which accepts
organizationId, enterpriseConnectionId, args:
GetEnterpriseConnectionTestRunsParams) and add the appropriate return type
annotation to the exported function signature.
🪄 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: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: d7b1d0a3-cb88-4036-8359-3dbbfd2ccab3
📒 Files selected for processing (19)
.changeset/wide-items-flow.mdpackages/clerk-js/src/core/resources/Organization.tspackages/clerk-js/src/core/resources/User.tspackages/clerk-js/src/core/resources/__tests__/Organization.test.tspackages/clerk-js/src/utils/enterpriseConnection.tspackages/shared/src/react/hooks/index.tspackages/shared/src/react/hooks/useOrganizationEnterpriseConnectionTestRuns.shared.tspackages/shared/src/react/hooks/useOrganizationEnterpriseConnectionTestRuns.tsxpackages/shared/src/react/hooks/useOrganizationEnterpriseConnections.shared.tspackages/shared/src/react/hooks/useOrganizationEnterpriseConnections.tsxpackages/shared/src/react/stable-keys.tspackages/shared/src/types/enterpriseConnection.tspackages/shared/src/types/organization.tspackages/shared/src/types/user.tspackages/ui/src/components/ConfigureSSO/ConfigureSSO.tsxpackages/ui/src/components/ConfigureSSO/ConfigureSSOContext.tsxpackages/ui/src/components/ConfigureSSO/__tests__/ConfigureSSO.test.tsxpackages/ui/src/components/ConfigureSSO/steps/ConfigureStep.tsxpackages/ui/src/components/ConfigureSSO/steps/TestConfigurationStep.tsx
| '@clerk/ui': minor | ||
| --- | ||
|
|
||
| Internal `<ConfigureSSO />` refactor to call new org-scoped enterprise connections FAPI endpoints, replacing the `/me/` deprecated scope. |
There was a problem hiding this comment.
Clarify that org-scoped methods are additions, not replacements.
The description states "replacing the /me/ deprecated scope," but per the PR objectives, the user-scoped enterprise connection methods and /v1/me/* paths are left intact. This PR adds organization-scoped methods alongside the existing user-scoped ones rather than replacing them. Consider rewording to avoid implying removal:
Internal
<ConfigureSSO />refactor to use new org-scoped enterprise connections FAPI endpoints alongside the existing/me/scope.
or
Internal
<ConfigureSSO />refactor to call new org-scoped enterprise connections FAPI endpoints. Adds Organization resource methods for enterprise connection management.
🤖 Prompt for 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.
In @.changeset/wide-items-flow.md at line 7, Update the changelog sentence about
the `<ConfigureSSO />` refactor to state that org-scoped enterprise connection
FAPI endpoints are being added alongside — not replacing — the existing
user-scoped `/v1/me/*` methods; specifically rephrase to mention that
organization-scoped methods (Organization resource methods) are added in
addition to the existing user-scoped `/me/` endpoints so it no longer implies
removal or deprecation of the `/me/` scope.
| getEnterpriseConnections: (params?: GetEnterpriseConnectionsParams) => Promise<EnterpriseConnectionResource[]>; | ||
| createEnterpriseConnection: ( | ||
| params: CreateOrganizationEnterpriseConnectionParams, | ||
| ) => Promise<EnterpriseConnectionResource>; | ||
| updateEnterpriseConnection: ( | ||
| enterpriseConnectionId: string, | ||
| params: UpdateOrganizationEnterpriseConnectionParams, | ||
| ) => Promise<EnterpriseConnectionResource>; | ||
| deleteEnterpriseConnection: (enterpriseConnectionId: string) => Promise<DeletedObjectResource>; | ||
| createEnterpriseConnectionTestRun: ( | ||
| enterpriseConnectionId: string, | ||
| ) => Promise<EnterpriseConnectionTestRunInitResource>; | ||
| getEnterpriseConnectionTestRuns: ( | ||
| enterpriseConnectionId: string, | ||
| params?: GetEnterpriseConnectionTestRunsParams, | ||
| ) => Promise<ClerkPaginatedResponse<EnterpriseConnectionTestRunResource>>; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add JSDoc documentation for the new public API methods.
These six enterprise connection methods are part of the public OrganizationResource API surface but lack individual JSDoc comments. Developers need documentation for parameters, return values, and usage examples.
📝 Example JSDoc for one method
+ /**
+ * Retrieves all enterprise connections for this organization.
+ *
+ * `@param` params - Optional parameters for filtering connections
+ * `@param` params.withOrganizationAccountLinking - When true, only return connections that support organization account linking
+ * `@returns` Promise resolving to an array of enterprise connection resources
+ * `@example`
+ * const connections = await org.getEnterpriseConnections({ withOrganizationAccountLinking: true });
+ */
getEnterpriseConnections: (params?: GetEnterpriseConnectionsParams) => Promise<EnterpriseConnectionResource[]>;Apply similar patterns to the other five methods.
As per coding guidelines, "All public APIs must be documented with JSDoc" and packages should "Maintain comprehensive JSDoc comments for public APIs".
🤖 Prompt for 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.
In `@packages/shared/src/types/organization.ts` around lines 79 - 94, Add JSDoc
comments for the six public OrganizationResource methods:
getEnterpriseConnections, createEnterpriseConnection,
updateEnterpriseConnection, deleteEnterpriseConnection,
createEnterpriseConnectionTestRun, and getEnterpriseConnectionTestRuns; for each
comment include a short description, `@param` annotations for each parameter
type/name (e.g., params, enterpriseConnectionId), an `@returns` describing the
Promise-wrapped return type (e.g., Promise<EnterpriseConnectionResource> or
ClerkPaginatedResponse<EnterpriseConnectionTestRunResource>), and a brief usage
example or note about optional params where applicable so the public API is
fully documented per guidelines.
LauraBeatris
force-pushed
the
iago/orgs-1597-sdk-replace-meenterprise_connections-with-organization
branch
from
6338def to
2f51809
Compare
LauraBeatris
force-pushed
the
iago/orgs-1597-sdk-replace-meenterprise_connections-with-organization
branch
from
02f04e8 to
36d6348
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/shared/src/react/hooks/useOrganizationEnterpriseConnectionTestRuns.tsx (1)
1-139: ⚡ Quick winAdd unit tests for this hook.
No test file for
useOrganizationEnterpriseConnectionTestRunsis present in this change set. The arm-until-first-row polling logic (shouldPoll, the disarm effect, andrevalidate()'s empty-list gating) is non-trivial and benefits from coverage of: polling armed only when the list is empty, polling disarmed once rows appear, theenabled: falsedormant path, andisPollingtransitions.Want me to scaffold a test file covering these cases?
As per coding guidelines: "If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes."
🤖 Prompt for 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. In `@packages/shared/src/react/hooks/useOrganizationEnterpriseConnectionTestRuns.tsx` around lines 1 - 139, Add a unit test file for the hook useOrganizationEnterpriseConnectionTestRuns that exercises the arm-until-first-row polling behavior: mock the organization returned by useOrganizationBase and its getEnterpriseConnectionTestRuns (and mock Clerk loaded/authenticated via useClerkInstanceContext and cache keys), then assert that calling revalidate() sets shouldPoll/isPolling only when the returned list is empty, that the useEffect disarms polling (shouldPoll -> false and isPolling -> false) when a subsequent fetch returns rows, that the hook is dormant when enabled: false (no fetch/no polling), and that isPolling transitions correctly; exercise the revalidate path to ensure queryClient.invalidateQueries is called and the gating (if !hasRows) is respected. Reference the hook name useOrganizationEnterpriseConnectionTestRuns, the revalidate callback, the shouldPoll state, and the isPolling return flag when writing tests.
🤖 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
`@packages/shared/src/react/hooks/useOrganizationEnterpriseConnectionTestRuns.tsx`:
- Around line 97-104: The refetchInterval currently keeps polling indefinitely
while shouldPoll is true and responses are empty/errors; update
useOrganizationEnterpriseConnectionTestRuns.tsx by adding a deterministic
stop/backoff: track a retry counter (e.g., useRef or internal variable) tied to
the query identified by useOrganizationEnterpriseConnectionTestRuns, increment
it on empty responses or errors, reset it when hasRows becomes true, and stop
polling (return false) after a configurable maxAttempts or apply exponential
backoff (multiply pollIntervalMs by 2^attempts up to a cap) in the
refetchInterval function; also ensure repeated errors immediately increment the
counter and cease polling after maxErrors, and apply the same change to
useEnterpriseConnectionTestRuns.tsx (referencing refetchInterval, shouldPoll,
pollIntervalMs, and refetchIntervalInBackground).
- Around line 91-96: The queryFn currently uses optional chaining
(organization?.getEnterpriseConnectionTestRuns) which lets it resolve to
undefined; change it to explicitly guard organization (e.g., if (!organization)
throw or return a rejected Promise) before calling
organization.getEnterpriseConnectionTestRuns so queryFn always returns a
Promise. Update the queryFn in useOrganizationEnterpriseConnectionTestRuns to
remove the optional chaining and call
organization.getEnterpriseConnectionTestRuns(enterpriseConnectionId,
fetchParams) only after confirming organization is present (and keep the
existing enterpriseConnectionId guard).
---
Nitpick comments:
In
`@packages/shared/src/react/hooks/useOrganizationEnterpriseConnectionTestRuns.tsx`:
- Around line 1-139: Add a unit test file for the hook
useOrganizationEnterpriseConnectionTestRuns that exercises the
arm-until-first-row polling behavior: mock the organization returned by
useOrganizationBase and its getEnterpriseConnectionTestRuns (and mock Clerk
loaded/authenticated via useClerkInstanceContext and cache keys), then assert
that calling revalidate() sets shouldPoll/isPolling only when the returned list
is empty, that the useEffect disarms polling (shouldPoll -> false and isPolling
-> false) when a subsequent fetch returns rows, that the hook is dormant when
enabled: false (no fetch/no polling), and that isPolling transitions correctly;
exercise the revalidate path to ensure queryClient.invalidateQueries is called
and the gating (if !hasRows) is respected. Reference the hook name
useOrganizationEnterpriseConnectionTestRuns, the revalidate callback, the
shouldPoll state, and the isPolling return flag when writing tests.
🪄 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: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: e63ec11b-0f19-4a8c-a903-8bb2b0f80b98
📒 Files selected for processing (18)
.changeset/wide-items-flow.mdpackages/clerk-js/src/core/resources/Organization.tspackages/clerk-js/src/core/resources/User.tspackages/clerk-js/src/core/resources/__tests__/Organization.test.tspackages/clerk-js/src/utils/enterpriseConnection.tspackages/shared/src/react/hooks/index.tspackages/shared/src/react/hooks/useOrganizationEnterpriseConnectionTestRuns.shared.tspackages/shared/src/react/hooks/useOrganizationEnterpriseConnectionTestRuns.tsxpackages/shared/src/react/hooks/useOrganizationEnterpriseConnections.shared.tspackages/shared/src/react/hooks/useOrganizationEnterpriseConnections.tsxpackages/shared/src/react/stable-keys.tspackages/shared/src/types/enterpriseConnection.tspackages/shared/src/types/organization.tspackages/shared/src/types/user.tspackages/ui/src/components/ConfigureSSO/ConfigureSSO.tsxpackages/ui/src/components/ConfigureSSO/ConfigureSSOContext.tsxpackages/ui/src/components/ConfigureSSO/__tests__/ConfigureSSO.test.tsxpackages/ui/src/components/ConfigureSSO/steps/TestConfigurationStep.tsx
✅ Files skipped from review due to trivial changes (3)
- packages/shared/src/react/stable-keys.ts
- .changeset/wide-items-flow.md
- packages/ui/src/components/ConfigureSSO/tests/ConfigureSSO.test.tsx
🚧 Files skipped from review as they are similar to previous changes (14)
- packages/ui/src/components/ConfigureSSO/ConfigureSSO.tsx
- packages/shared/src/types/user.ts
- packages/shared/src/react/hooks/useOrganizationEnterpriseConnectionTestRuns.shared.ts
- packages/shared/src/react/hooks/useOrganizationEnterpriseConnections.shared.ts
- packages/shared/src/types/organization.ts
- packages/clerk-js/src/utils/enterpriseConnection.ts
- packages/clerk-js/src/core/resources/User.ts
- packages/shared/src/react/hooks/useOrganizationEnterpriseConnections.tsx
- packages/clerk-js/src/core/resources/tests/Organization.test.ts
- packages/ui/src/components/ConfigureSSO/steps/TestConfigurationStep.tsx
- packages/ui/src/components/ConfigureSSO/ConfigureSSOContext.tsx
- packages/shared/src/types/enterpriseConnection.ts
- packages/clerk-js/src/core/resources/Organization.ts
- packages/shared/src/react/hooks/index.ts
| queryFn: () => { | ||
| if (!enterpriseConnectionId) { | ||
| throw new Error('enterpriseConnectionId is required to fetch test runs'); | ||
| } | ||
| return organization?.getEnterpriseConnectionTestRuns(enterpriseConnectionId, fetchParams); | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -e ts -e tsx 'useQuery' --full-path 'packages/shared/src/react/query' --exec cat {}Repository: clerk/javascript
Length of output: 1540
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the hook implementation around the referenced lines
hook="packages/shared/src/react/hooks/useOrganizationEnterpriseConnectionTestRuns.tsx"
echo "---- $hook (lines ~70-120) ----"
nl -ba "$hook" | sed -n '70,130p'
# Locate and print the query wrapper implementation files
fd -e ts -e tsx 'useBaseQuery|queryOptions|types' packages/shared/src/react/query --full-path > /tmp/query_files.txt
echo "---- query wrapper files ----"
cat /tmp/query_files.txt
echo "---- useBaseQuery.ts ----"
nl -ba "$(rg -l 'useBaseQuery' packages/shared/src/react/query -g'*.ts' -g'*.tsx' | head -n 5 | head -n 1)" | sed -n '1,200p'
echo "---- types.ts (if exists) ----"
types_file="$(fd -a -e ts -e tsx 'types' packages/shared/src/react/query | head -n 1 || true)"
if [ -n "${types_file:-}" ]; then
nl -ba "$types_file" | sed -n '1,220p'
else
echo "No types file found by name under packages/shared/src/react/query"
fi
echo "---- queryOptions.ts (if exists) ----"
qo_file="$(fd -a -e ts -e tsx 'queryOptions' packages/shared/src/react/query | head -n 1 || true)"
if [ -n "${qo_file:-}" ]; then
nl -ba "$qo_file" | sed -n '1,240p'
else
echo "No queryOptions file found by name under packages/shared/src/react/query"
fiRepository: clerk/javascript
Length of output: 227
🌐 Web query:
TanStack Query queryFn returns undefined runtime error "queryFn resolved to undefined"
💡 Result:
In TanStack Query (starting from v4), the query function (queryFn) must not return undefined [1][2]. This is a strict requirement because the library uses undefined internally to represent a state where data is not yet available or has not been fetched [3][2]. Returning undefined causes the query to transition into an error state [4][5], which can lead to UI bugs or infinite fetching loops if the query is re-triggered by component re-renders [4]. To resolve the "queryFn resolved to undefined" error, you must ensure that your query function always returns a defined value or throws an error [6][7]. Recommended strategies: 1. Use the enabled option: If you want to skip fetching when certain conditions are not met (e.g., an ID is missing), do not run the query at all [2]. Use the enabled property to prevent the query function from executing until the necessary dependencies are ready [2]. useQuery({ queryKey: ['user', userId], queryFn: => fetchUser(userId!), enabled:!!userId, // Query only runs when userId is defined }) 2. Return null instead of undefined: If the query executes successfully but there is no data to return, return null [6][7]. TanStack Query treats null as a valid successful result, whereas undefined is treated as a failure [6][7]. 3. Explicitly handle empty responses: If your API client (like axios or fetch) returns undefined for certain cases (e.g., a 404), explicitly catch that case within your queryFn and return null or throw an error depending on your requirements [4][8]. queryFn: async => { const data = await fetchData; return data?? null; // Ensure a defined value is returned } For more details, consult the official TanStack Query documentation on query functions [6][7].
Citations:
- 1: Undefined not allowed as return value of queryFn since v4 TanStack/query#4964
- 2: https://zenn.dev/takoyoki/articles/06d5e5805fba44?locale=en
- 3: queryFn returning `undefined` should be cached TanStack/query#6029
- 4: Query that returns undefined is stuck in a fetching loop when used in child and parent component TanStack/query#4910
- 5: https://www.answeroverflow.com/m/1243328274592174140
- 6: https://tanstack.com/query/latest/docs/framework/react/guides/query-functions
- 7: https://tanstack.com/query/v5/docs/framework/react/guides/query-functions
- 8: https://stackoverflow.com/questions/75163140/query-data-cannot-be-undefined-please-make-sure-to-return-a-value-other-than-un
🏁 Script executed:
#!/bin/bash
# Search within the codebase for any mention of "resolved to undefined" or similar runtime checks.
rg -n "resolved to undefined|queryFn.*undefined|undefined.*queryFn" packages -S || trueRepository: clerk/javascript
Length of output: 223
🏁 Script executed:
#!/bin/bash
set -euo pipefail
hook="packages/shared/src/react/hooks/useOrganizationEnterpriseConnectionTestRuns.tsx"
echo "---- $hook (lines ~70-130) ----"
nl -ba "$hook" | sed -n '70,130p'
echo "---- query wrapper files ----"
fd -e ts -e tsx 'useBaseQuery|queryOptions|types' packages/shared/src/react/query --full-path > /tmp/query_files.txt
cat /tmp/query_files.txt
echo "---- useBaseQuery.ts ----"
use_base="$(rg -l 'useBaseQuery' packages/shared/src/react/query -g'*.ts' -g'*.tsx' | head -n 1)"
nl -ba "$use_base" | sed -n '1,240p'
echo "---- types.ts (best match) ----"
types_file="$(fd -a -e ts -e tsx 'types' packages/shared/src/react/query | head -n 1 || true)"
if [ -n "${types_file:-}" ]; then
nl -ba "$types_file" | sed -n '1,260p'
else
echo "No types file found by name under packages/shared/src/react/query"
fi
echo "---- queryOptions.ts (best match) ----"
qo_file="$(fd -a -e ts -e tsx 'queryOptions' packages/shared/src/react/query | head -n 1 || true)"
if [ -n "${qo_file:-}" ]; then
nl -ba "$qo_file" | sed -n '1,320p'
else
echo "No queryOptions file found by name under packages/shared/src/react/query"
fiRepository: clerk/javascript
Length of output: 227
🌐 Web query:
TanStack Query "queryFn" returns undefined runtime error "resolves to undefined"
💡 Result:
In TanStack Query, queryFn is not allowed to resolve to undefined. If your queryFn returns undefined (or implicitly returns undefined), TanStack Query treats it as a failure and logs/throws the runtime warning/error “Query data cannot be undefined. Please make sure to return a value other than undefined from your query function.” [1][2][3] What this means in practice 1) Ensure every success path returns a value. - TanStack Query docs state: “On success, the resolved value may be anything except undefined… Queries that resolve to undefined will be treated as failed.” [1][2] - The docs also state the query function must either resolve data or throw an error. [1][2] 2) Don’t “early return;” / implicit return from queryFn. - If you forget to return from an async function (e.g., you have a.then(...) without returning it, or you omit return), JavaScript will return undefined implicitly, causing this error. [3] 3) If “no data” is a valid successful outcome, return null (or another non-undefined sentinel). - Docs explicitly recommend: “To store ‘nothing’ as a successful result in the query cache, resolve null instead.” [1][2] - TanStack maintainers likewise recommend transforming “possibly undefined entity” results to null rather than returning undefined. [4] 4) If the query should not run when some condition isn’t met, use enabled (don’t return undefined). - Returning undefined makes it impossible to distinguish “not executed” vs “fetched but undefined”; the correct pattern is to disable the query via enabled. [5] Common fixes (patterns) A) Return null instead of undefined - Example approach: when your backend lookup may not find a record, do return result ?? null rather than returning undefined. [4] B) Throw an error instead of returning undefined (if “not found” is truly an error) - If “missing record” should be treated as failure, throw (or return a rejected Promise) so the query goes to error state instead of resolving undefined. [1][2] C) Use enabled to guard execution - If userId (or another prerequisite) is missing, set enabled: !!userId and make the queryFn assume it exists; don’t return undefined. [5] D) Fix missing returns - If your axios/fetch call is inside a promise chain, ensure you return the data from queryFn (otherwise it resolves undefined). [3] Why you see it often - TanStack Query uses undefined internally as a special “no data / no initial data” signal; therefore it cannot represent successful cached data as undefined. [6][7] If you share the specific code of your queryFn and how you handle “not found / empty response,” I can rewrite it in the correct style (return null vs throw vs enabled) for your exact case.
Citations:
- 1: https://tanstack.com/query/latest/docs/framework/react/guides/query-functions
- 2: https://tanstack.com/query/v5/docs/framework/react/guides/query-functions
- 3: https://stackoverflow.com/questions/75163140/query-data-cannot-be-undefined-please-make-sure-to-return-a-value-other-than-un
- 4: Best practice for returning undefined data TanStack/query#7020
- 5: https://zenn.dev/takoyoki/articles/06d5e5805fba44?locale=en
- 6: queryFn returning `undefined` should be cached TanStack/query#6029
- 7: Undefined not allowed as return value of queryFn since v4 TanStack/query#4964
🏁 Script executed:
#!/bin/bash
rg -n "resolved to undefined|queryFn.*undefined|undefined.*queryFn" packages -S || trueRepository: clerk/javascript
Length of output: 223
🏁 Script executed:
#!/bin/bash
set -euo pipefail
hook="packages/shared/src/react/hooks/useOrganizationEnterpriseConnectionTestRuns.tsx"
echo "---- $hook (80-120) ----"
sed -n '80,120p' "$hook"
echo "---- locate useBaseQuery ----"
use_base="$(fd -e ts -e tsx 'useBaseQuery' packages/shared/src/react/query -t f | head -n 5 | tr '\n' ' ')"
echo "$use_base"
base_file="$(fd -e ts -e tsx 'useBaseQuery' packages/shared/src/react/query -t f | head -n 1)"
echo "---- $base_file (1-260) ----"
sed -n '1,260p' "$base_file"
echo "---- queryOptions.ts (1-120) ----"
qo="$(fd -a -e ts -e tsx 'queryOptions.ts' packages/shared/src/react/query | head -n 1 || true)"
if [ -n "${qo:-}" ]; then
sed -n '1,160p' "$qo"
fi
echo "---- types in packages/shared/src/react/query ----"
fd -a -e ts -e tsx -t f packages/shared/src/react/query | rg -n 'types|useBaseQuery|queryOptions|useClerkQuery' || trueRepository: clerk/javascript
Length of output: 8379
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Also check whether any wrapper transforms undefined to null, or sets select/initialData
rg -n "initialData|select:|undefined|SkipToken|queryFn" packages/shared/src/react/query -SRepository: clerk/javascript
Length of output: 6184
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect useBaseQuery implementation around where queryFn is forwarded
base_file="packages/shared/src/react/query/useBaseQuery.ts"
echo "---- $base_file (1-220) ----"
sed -n '1,220p' "$base_file"
# Inspect use-clerk-query-client.ts
client_file="packages/shared/src/react/query/use-clerk-query-client.ts"
echo "---- $client_file (1-220) ----"
sed -n '1,220p' "$client_file"
# Find the type/implementation of getEnterpriseConnectionTestRuns
echo "---- Searching for getEnterpriseConnectionTestRuns ----"
rg -n "getEnterpriseConnectionTestRuns" packages/shared/src -S || true
rg -n "getEnterpriseConnectionTestRuns" packages -S || trueRepository: clerk/javascript
Length of output: 7257
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the hook file fully around queryEnabled/queryFn
hook="packages/shared/src/react/hooks/useOrganizationEnterpriseConnectionTestRuns.tsx"
echo "---- $hook (70-120) ----"
sed -n '70,130p' "$hook"
echo "---- $hook (1-120) imports/types (top) ----"
sed -n '1,80p' "$hook"
# Inspect types for the method in shared types
echo "---- packages/shared/src/types/organization.ts (70-120) ----"
sed -n '70,130p' packages/shared/src/types/organization.ts
echo "---- packages/shared/src/types/user.ts (120-190) ----"
sed -n '120,200p' packages/shared/src/types/user.ts
# Inspect implementation in clerk-js core resources
org_impl="packages/clerk-js/src/core/resources/Organization.ts"
user_impl="packages/clerk-js/src/core/resources/User.ts"
echo "---- $org_impl (220-280) ----"
sed -n '220,280p' "$org_impl"
echo "---- $user_impl (360-450) ----"
sed -n '360,450p' "$user_impl"
# Inspect useQuery wrapper/typing in shared
echo "---- packages/shared/src/react/query/useQuery.ts (1-220) ----"
usequery="packages/shared/src/react/query/useQuery.ts"
if [ -f "$usequery" ]; then sed -n '1,240p' "$usequery"; else echo "useQuery.ts not found"; fi
echo "---- packages/shared/src/react/query/types.ts (if exists) ----"
typesf="packages/shared/src/react/query/types.ts"
if [ -f "$typesf" ]; then sed -n '1,260p' "$typesf"; else echo "types.ts not found"; fi
echo "---- packages/shared/src/react/query/useBaseQuery.ts types import (UseBaseQueryOptions) ----"
rg -n "export type UseBaseQueryOptions|interface UseBaseQueryOptions|UseBaseQueryOptions" packages/shared/src/react/query -SRepository: clerk/javascript
Length of output: 20699
Narrow queryFn to never resolve undefined
TanStack Query requires queryFn not to resolve to undefined. This hook’s organization?.getEnterpriseConnectionTestRuns(...) optional chaining widens the queryFn return type to include undefined (even though queryEnabled gates execution on Boolean(organization) and getEnterpriseConnectionTestRuns always returns a Promise in both shared types and the clerk-js implementation). Remove optional chaining / add an explicit organization guard to keep the return type aligned with TanStack Query’s invariant.
♻️ Proposed guard
queryFn: () => {
- if (!enterpriseConnectionId) {
- throw new Error('enterpriseConnectionId is required to fetch test runs');
+ if (!organization) {
+ throw new Error('An active organization is required to fetch test runs');
}
- return organization?.getEnterpriseConnectionTestRuns(enterpriseConnectionId, fetchParams);
+ if (!enterpriseConnectionId) {
+ throw new Error('enterpriseConnectionId is required to fetch test runs');
+ }
+ return organization.getEnterpriseConnectionTestRuns(enterpriseConnectionId, fetchParams);
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| queryFn: () => { | |
| if (!enterpriseConnectionId) { | |
| throw new Error('enterpriseConnectionId is required to fetch test runs'); | |
| } | |
| return organization?.getEnterpriseConnectionTestRuns(enterpriseConnectionId, fetchParams); | |
| }, | |
| queryFn: () => { | |
| if (!organization) { | |
| throw new Error('An active organization is required to fetch test runs'); | |
| } | |
| if (!enterpriseConnectionId) { | |
| throw new Error('enterpriseConnectionId is required to fetch test runs'); | |
| } | |
| return organization.getEnterpriseConnectionTestRuns(enterpriseConnectionId, fetchParams); | |
| }, |
🤖 Prompt for 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.
In
`@packages/shared/src/react/hooks/useOrganizationEnterpriseConnectionTestRuns.tsx`
around lines 91 - 96, The queryFn currently uses optional chaining
(organization?.getEnterpriseConnectionTestRuns) which lets it resolve to
undefined; change it to explicitly guard organization (e.g., if (!organization)
throw or return a rejected Promise) before calling
organization.getEnterpriseConnectionTestRuns so queryFn always returns a
Promise. Update the queryFn in useOrganizationEnterpriseConnectionTestRuns to
remove the optional chaining and call
organization.getEnterpriseConnectionTestRuns(enterpriseConnectionId,
fetchParams) only after confirming organization is present (and keep the
existing enterpriseConnectionId guard).
| refetchInterval: q => { | ||
| if (!shouldPoll) { | ||
| return false; | ||
| } | ||
|
|
||
| const hasRows = (q.state.data?.data?.length ?? 0) > 0; | ||
| return hasRows ? false : pollIntervalMs; | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd 'EnterpriseConnection' --extension tsx --extension ts -p 'packages/shared/src/react/hooks' --exec rg -n 'refetchInterval|maxAttempts|attempts|shouldPoll|pollInterval' {}Repository: clerk/javascript
Length of output: 912
🏁 Script executed:
#!/bin/bash
# Find the sibling hook and any polling bounds/backoff/attempt caps
rg -n "useOrganizationEnterpriseConnections|refetchInterval|maxAttempts|attempts|pollInterval|shouldPoll|backoff" packages/shared/src/react/hooks --glob='*EnterpriseConnection*' --glob='*useOrganization*'Repository: clerk/javascript
Length of output: 3205
🏁 Script executed:
#!/bin/bash
set -euo pipefail
files=(
packages/shared/src/react/hooks/useOrganizationEnterpriseConnections.tsx
packages/shared/src/react/hooks/useOrganizationEnterpriseConnections.shared.ts
packages/shared/src/react/hooks/useEnterpriseConnectionTestRuns.tsx
packages/shared/src/react/hooks/useOrganizationEnterpriseConnectionTestRuns.tsx
)
for f in "${files[@]}"; do
echo "===== $f ====="
if [ -f "$f" ]; then
# show the top 220 lines (or full if shorter) to catch polling/error handling logic
n=$(wc -l < "$f")
if [ "$n" -gt 220 ]; then
sed -n '1,220p' "$f" | nl -ba | sed -n '1,220p'
else
cat -n "$f"
fi
else
echo "MISSING: $f"
fi
echo
done
# Focused extraction: any mention of refetchInterval/shouldPoll/pollInterval/maxAttempts/backoff/attempt
for f in "${files[@]}"; do
echo "===== matches in $f ====="
rg -n "refetchInterval|maxAttempts|attempts|backoff|shouldPoll|pollInterval" "$f" || true
echo
doneRepository: clerk/javascript
Length of output: 18059
Add a deterministic stop/backoff to test-run polling (empty/error => infinite requests)
useOrganizationEnterpriseConnectionTestRuns.tsx arms polling via shouldPoll, and refetchInterval keeps returning pollIntervalMs as long as shouldPoll is true and the response has no rows—shouldPoll is only cleared once hasRows becomes true. If the query repeatedly errors or keeps returning an empty list, polling continues indefinitely (it only avoids background-tab refetching via refetchIntervalInBackground: false). The same pattern exists in useEnterpriseConnectionTestRuns.tsx. Consider stopping after N attempts, stopping on repeated errors, and/or applying exponential backoff.
🤖 Prompt for 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.
In
`@packages/shared/src/react/hooks/useOrganizationEnterpriseConnectionTestRuns.tsx`
around lines 97 - 104, The refetchInterval currently keeps polling indefinitely
while shouldPoll is true and responses are empty/errors; update
useOrganizationEnterpriseConnectionTestRuns.tsx by adding a deterministic
stop/backoff: track a retry counter (e.g., useRef or internal variable) tied to
the query identified by useOrganizationEnterpriseConnectionTestRuns, increment
it on empty responses or errors, reset it when hasRows becomes true, and stop
polling (return false) after a configurable maxAttempts or apply exponential
backoff (multiply pollIntervalMs by 2^attempts up to a cap) in the
refetchInterval function; also ensure repeated errors immediately increment the
counter and cease polling after maxErrors, and apply the same change to
useEnterpriseConnectionTestRuns.tsx (referencing refetchInterval, shouldPoll,
pollIntervalMs, and refetchIntervalInBackground).
API Changes Report
Summary
@clerk/sharedCurrent version: 4.14.0 Subpath
|
…anization resource
- Add 6 methods on Organization.ts hitting /organizations/{id}/enterprise_connections/*
- Add new param types CreateOrganizationEnterpriseConnectionParams + Update variant; deprecate Me* aliases
- Re-route shared types to canonical Organization* names
- Extract toEnterpriseConnectionBody helper into utils/enterpriseConnection; used by User and Organization
- Add Organization.test.ts coverage mirroring User.test.ts patterns
User.ts methods + /me/* paths kept untouched (deprecated server-side but wired).
…ions - Add __internal_useOrganizationEnterpriseConnections + test-runs hook in @clerk/shared/react - Swap all user.*EnterpriseConnection* call sites in <ConfigureSSO /> to organization.* - Preserve useReverification wrapping - Update tests + fixtures to reflect the new call surface ORGS-1597 — backend org-scoped paths (clerk_go#19109) ship the canonical surface; this routes the SDK consumer to them.
- Collapse the two per-package changesets into a single auto-named one - Reword as internal refactor (endpoints aren't public consumer surface) - Update one-connection comment from "per user" to "per organization"
iagodahlem
force-pushed
the
iago/orgs-1597-sdk-replace-meenterprise_connections-with-organization
branch
from
36d6348 to
0d8f86e
Compare
jacekradko
deleted the
iago/orgs-1597-sdk-replace-meenterprise_connections-with-organization
branch
3 participants
Description
Backend clerk_go#19109 (merged) promoted the per-org enterprise_connections surface to
/v1/organizations/{org_id}/enterprise_connections/*. The/v1/me/enterprise_connections/*paths stay wired but are markeddeprecated: truein OpenAPI.This PR routes the JS SDK consumers through the canonical org-scoped paths.
What changed
@clerk/clerk-jsgetEnterpriseConnections,createEnterpriseConnection,updateEnterpriseConnection,deleteEnterpriseConnection,createEnterpriseConnectionTestRun,getEnterpriseConnectionTestRunson theOrganizationresource. Paths hit/v1/organizations/{org_id}/enterprise_connections/*;organization_idis omitted from the request body since the URL path is authoritative.User.tsintosrc/utils/enterpriseConnection.ts(toEnterpriseConnectionBody) and reused it from bothUser.tsandOrganization.tsso the two surfaces can never drift.User.tsmethods + their/me/*calls are unchanged.@clerk/sharedMeprefix (CreateOrganizationEnterpriseConnectionParams,UpdateOrganizationEnterpriseConnectionParams, provider/SAML/OIDC inputs). The oldMe*names are kept as@deprecatedaliases for backwards compatibility.OrganizationResourcemethod signatures for the six new methods.@clerk/shared/react:__internal_useOrganizationEnterpriseConnectionsand__internal_useOrganizationEnterpriseConnectionTestRuns. They mirror the existing user-scoped hooks but resolve the active organization viauseOrganization().@clerk/ui<ConfigureSSO />now consumes the org-scoped hooks. The provider passesorganization.*methods down through context, so the step components (ConfigureStep,ConfirmationStep,TestConfigurationStep,SelectProviderStep) consume the new surface without per-step rewires.useReverificationwrapping is preserved on every sensitive mutation.TestConfigurationSteppreviously calleduser.*directly for test-run reads/writes; it now usesuseOrganization()to reach the same methods onorganization.What stays the same
user.*EnterpriseConnection*methods and their/v1/me/enterprise_connections/*paths are untouched. External SDK consumers that already use them continue to work; backend keeps the deprecated paths wired.organization.*EnterpriseConnection*methods. The type namesCreateMeEnterpriseConnectionParamsetc. are now `@deprecated` aliases.Test plan
Linear
ORGS-1597
Out of scope
Checklist
Type of change