Skip to content

chore: add identity package#648

Open
khendrikse wants to merge 8 commits intomainfrom
EX-1826/add-identity-package
Open

chore: add identity package#648
khendrikse wants to merge 8 commits intomainfrom
EX-1826/add-identity-package

Conversation

@khendrikse
Copy link
Copy Markdown

@khendrikse khendrikse commented Apr 13, 2026

  • Moves the identity package into a 'prod' folder in the packages directory
  • Adds provenance to release process because with a public repo this is now possible
  • Added suppressions for some type errors. A good fix would require a larger overhaul and I do not want to do that in this PR

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 13, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a new identity package at packages/identity/prod with package manifest, build (tsup, tsconfig), test (vitest) and lint suppressions. Introduces TypeScript source modules implementing auth, admin, account, user, refresh, environment, config, events, errors, fetch, nextjs, cookies, types, and a public main export. Adds global typings, many browser/server Vitest tests and fixtures, workspace and release-please config entries, CI publish step, .gitignore files, and a CODEOWNERS rule for /packages/identity/**/*.

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'chore: add identity package' accurately captures the main change: moving/adding the identity package into a prod folder within the packages directory.
Description check ✅ Passed The description is directly related to the changeset, explaining the main objectives: moving the identity package into a prod folder, adding release provenance, and suppressing type errors.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch EX-1826/add-identity-package

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

@khendrikse khendrikse force-pushed the EX-1826/add-identity-package branch from c971c47 to 1359464 Compare April 13, 2026 15:00
@khendrikse khendrikse marked this pull request as ready for review April 17, 2026 15:28
@khendrikse khendrikse requested a review from a team as a code owner April 17, 2026 15:28
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🧹 Nitpick comments (9)
packages/identity/prod/test/admin.browser.test.ts (1)

24-71: Reduce repetitive error-assertion boilerplate in admin browser tests.
Line 25 through Line 70 repeat the same import + catch + assertion pattern; extracting a helper will make these tests easier to maintain.

♻️ Suggested refactor
 describe('admin (browser)', () => {
+  const expectServerOnlyAuthError = async (run: () => Promise<unknown>) => {
+    const { AuthError } = await import('../src/errors.js')
+    const error = await run().catch((e: unknown) => e)
+    expect(error).toBeInstanceOf(AuthError)
+    expect((error as InstanceType<typeof AuthError>).message).toContain('server-only')
+  }
+
   it('admin.listUsers throws AuthError in the browser', async () => {
     const { admin } = await import('../src/admin.js')
-    const { AuthError } = await import('../src/errors.js')
-
-    const error = await admin.listUsers().catch((e: unknown) => e)
-    expect(error).toBeInstanceOf(AuthError)
-    expect((error as InstanceType<typeof AuthError>).message).toContain('server-only')
+    await expectServerOnlyAuthError(() => admin.listUsers())
   })
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/identity/prod/test/admin.browser.test.ts` around lines 24 - 71,
Extract the repeated import/catch/assert pattern into a small helper in the test
file (e.g. async function assertAdminAuthError(run: () => Promise<unknown>)) and
use it for each case instead of duplicating code; the helper should import
AuthError once, call the provided admin method (admin.listUsers, admin.getUser,
admin.createUser, admin.updateUser, admin.deleteUser) capturing any thrown
value, assert it is an instance of AuthError and that its message contains
'server-only'. Replace each test body to call this helper with a lambda invoking
the corresponding admin.* method to remove the boilerplate.
packages/identity/prod/test/config.test.ts (1)

77-81: Double await on getSettings() may cause flaky tests.

Lines 79 and 80 both call getSettings() independently. If the module or mock state changes between calls, the second assertion might not throw the same error. Consider capturing the error once:

 it('throws MissingIdentityError when no client is available', async () => {
   const { getSettings, MissingIdentityError } = await import('../src/main.js')
-  await expect(getSettings()).rejects.toThrow(MissingIdentityError)
-  await expect(getSettings()).rejects.toThrow('Netlify Identity is not available')
+  const error = await getSettings().catch((e: unknown) => e)
+  expect(error).toBeInstanceOf(MissingIdentityError)
+  expect((error as Error).message).toBe('Netlify Identity is not available')
 })
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/identity/prod/test/config.test.ts` around lines 77 - 81, The test
calls getSettings() twice which can make it flaky; call getSettings() once and
capture its rejection (either via await
expect(getSettings()).rejects.toThrow(MissingIdentityError) combined in one
assertion or by awaiting getSettings() in a try/catch and asserting the caught
error is an instance of MissingIdentityError and its message contains 'Netlify
Identity is not available'); update the spec that references getSettings and
MissingIdentityError to use the single captured error for both checks.
packages/identity/prod/src/cookies.ts (1)

19-41: Refresh token stored without httpOnly flag—intentional but worth documenting.

Both nf_jwt and nf_refresh cookies are set with httpOnly: false. The inline comment explains that browser-side hydration needs to read nf_refresh via document.cookie. This is a reasonable trade-off for the architecture, but storing refresh tokens accessible to JavaScript increases XSS impact. Consider adding a brief note in the public API docs or README about this design decision and the importance of XSS mitigations.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/identity/prod/src/cookies.ts` around lines 19 - 41, Document the
intentional choice to set both NF_JWT_COOKIE and NF_REFRESH_COOKIE with
httpOnly: false in the public API docs/README and in any consumer-facing
guidance: mention that setAuthCookies uses cookies.set to expose nf_refresh to
document.cookie for browser-side hydration (backgroundHydrate/hydrateSession),
explain the XSS risk of storing a refresh token accessible to JavaScript, and
add recommended mitigations (e.g., CSP, input sanitization, dependency hygiene,
automated scanning) and a clear note that this trade-off is deliberate so
integrators are aware when using setAuthCookies and NF_REFRESH_COOKIE.
packages/identity/prod/src/refresh.ts (1)

153-159: URL construction assumes valid base URL.

Line 155 uses new URL(IDENTITY_PATH, globalThis.Netlify.context.url) which will throw if context.url is malformed. While unlikely in the Netlify runtime, this could cause an unhandled exception instead of a meaningful AuthError.

🛡️ Proposed fix to catch URL construction errors
   const ctx = getIdentityContext()
-  const identityUrl =
-    ctx?.url ?? (globalThis.Netlify?.context?.url ? new URL(IDENTITY_PATH, globalThis.Netlify.context.url).href : null)
+  let identityUrl = ctx?.url ?? null
+  if (!identityUrl && globalThis.Netlify?.context?.url) {
+    try {
+      identityUrl = new URL(IDENTITY_PATH, globalThis.Netlify.context.url).href
+    } catch {
+      // Malformed URL falls through to the error below
+    }
+  }
 
   if (!identityUrl) {
     throw new AuthError('Could not determine the Identity endpoint URL for token refresh')
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/identity/prod/src/refresh.ts` around lines 153 - 159, The current
identity URL assignment can throw if globalThis.Netlify.context.url is
malformed; wrap the URL construction in a try/catch around the new
URL(IDENTITY_PATH, globalThis.Netlify.context.url) so that any URIError or other
exception is caught and rethrown as an AuthError with a clear message (include
the original error message for debugging) before assigning identityUrl; update
the logic around getIdentityContext(), IDENTITY_PATH and identityUrl to use this
safe construction and ensure the final thrown error remains an AuthError when
URL construction fails.
packages/identity/prod/src/auth.ts (1)

387-418: Use fetchWithTimeout for consistency in email change callback.

handleEmailChangeCallback uses bare fetch (line 396), while other auth operations use fetchWithTimeout. This could leave the request hanging indefinitely if the server is unresponsive.

♻️ Suggested fix
-  const emailChangeRes = await fetch(`${identityUrl}/user`, {
+  const emailChangeRes = await fetchWithTimeout(`${identityUrl}/user`, {
     method: 'PUT',
     headers: {
       'Content-Type': 'application/json',
       Authorization: `Bearer ${jwt}`,
     },
     body: JSON.stringify({ email_change_token: emailChangeToken }),
   })
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/identity/prod/src/auth.ts` around lines 387 - 418, Replace the bare
fetch call in handleEmailChangeCallback with the project's fetchWithTimeout
helper to match other auth requests and avoid hanging; call
fetchWithTimeout(`${identityUrl}/user`, { method: 'PUT', headers: {
'Content-Type': 'application/json', Authorization: `Bearer ${jwt}` }, body:
JSON.stringify({ email_change_token: emailChangeToken }) }) using the same
timeout/opts pattern used elsewhere, preserve the current response/error
handling (checking emailChangeRes.ok, parsing json into
GoTrueErrorBody/UserData, throwing AuthError with status), and ensure
emitAuthEvent(AUTH_EVENTS.USER_UPDATED, user), clearHash(), and the returned {
type: 'email_change', user } remain unchanged.
packages/identity/prod/src/environment.ts (1)

20-37: Consider consolidating URL resolution logic.

discoverApiUrl() (lines 20-37) and getIdentityContext() (lines 75-95) both implement similar URL discovery fallbacks. This duplication could lead to inconsistencies if one is updated without the other.

Consider having discoverApiUrl delegate to getIdentityContext for the server path:

♻️ Suggested refactor
 const discoverApiUrl = (): string | null => {
   if (cachedApiUrl !== undefined) return cachedApiUrl

   if (isBrowser()) {
     cachedApiUrl = `${window.location.origin}${IDENTITY_PATH}`
   } else {
-    const identityContext = getIdentityContext()
-    if (identityContext?.url) {
-      cachedApiUrl = identityContext.url
-    } else if (globalThis.Netlify?.context?.url) {
-      cachedApiUrl = new URL(IDENTITY_PATH, globalThis.Netlify.context.url).href
-    } else if (typeof process !== 'undefined' && process.env?.URL) {
-      cachedApiUrl = new URL(IDENTITY_PATH, process.env.URL).href
-    }
+    cachedApiUrl = getIdentityContext()?.url ?? null
   }

   return cachedApiUrl ?? null
 }

Also applies to: 75-95

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/identity/prod/src/environment.ts` around lines 20 - 37, Refactor
discoverApiUrl to remove duplicated server-side URL fallbacks by delegating
server resolution to getIdentityContext: keep the early cachedApiUrl and browser
branch (isBrowser + window.location.origin + IDENTITY_PATH) in discoverApiUrl,
but for the non-browser path call getIdentityContext() and use
identityContext?.url (or construct with IDENTITY_PATH from the returned context
when present); remove the duplicated Netlify/process.env URL checks from
discoverApiUrl and ensure getIdentityContext continues to handle
globalThis.Netlify and process.env.URL logic so cachedApiUrl and functions
discoverApiUrl, getIdentityContext, IDENTITY_PATH, isBrowser, and cachedApiUrl
remain consistent.
packages/identity/prod/test/account.browser.test.ts (1)

224-236: Redundant cookie reset at line 235.

The cookie is already cleared at line 229, and is cleared again at line 235. Since clearBrowserAuthCookies() runs in afterEach, the explicit reset at line 235 appears unnecessary.

♻️ Suggested cleanup
   it('throws AuthError when no user is logged in', async () => {
     const { updateUser } = await import('../src/account.js')
     const { AuthError } = await import('../src/errors.js')
     mockCurrentUser.mockReturnValue(null)

     Object.defineProperty(document, 'cookie', { writable: true, value: '' })

     const error = await updateUser({ data: { full_name: 'New Name' } }).catch((e: unknown) => e)
     expect(error).toBeInstanceOf(AuthError)
     expect(error.message).toBe('No user is currently logged in')
-
-    Object.defineProperty(document, 'cookie', { writable: true, value: '' })
   })
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/identity/prod/test/account.browser.test.ts` around lines 224 - 236,
Remove the redundant cookie reset at the end of the test: the test already sets
document.cookie to '' before calling updateUser and clearBrowserAuthCookies() is
executed in afterEach, so delete the second Object.defineProperty(document,
'cookie', { writable: true, value: '' }) near the end of the 'throws AuthError
when no user is logged in' test (references: updateUser, mockCurrentUser,
document.cookie, clearBrowserAuthCookies, afterEach).
packages/identity/prod/src/admin.ts (1)

51-74: Content-Type: application/json header sent on all requests including DELETE.

While not harmful, DELETE requests typically don't have a body. The Content-Type header is unnecessary for deleteUser. This is a minor inconsistency but doesn't affect functionality.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/identity/prod/src/admin.ts` around lines 51 - 74, The adminFetch
function always adds 'Content-Type: application/json' which is unnecessary for
requests without a body (e.g., deleteUser). Update adminFetch to only set the
Content-Type header when the request has a body (e.g., when options.body is
present) or when options.method is not 'DELETE' (check options.method
case-insensitively); keep the other headers and Authorization behavior unchanged
so existing calls still work.
packages/identity/prod/src/types.ts (1)

145-150: Tighten CreateUserParams.data to the documented whitelist.

The docs say only role, app_metadata, and user_metadata are accepted, but Record<string, unknown> allows anything at compile time.

Proposed type tightening
+export interface CreateUserData {
+  role?: string
+  app_metadata?: Record<string, unknown>
+  user_metadata?: Record<string, unknown>
+}
+
 export interface CreateUserParams {
   email: string
   password: string
   /** Identity user fields: `role`, `app_metadata`, `user_metadata`. Other keys are ignored. */
-  data?: Record<string, unknown>
+  data?: CreateUserData
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/identity/prod/src/types.ts` around lines 145 - 150,
CreateUserParams.data is too permissive; replace the Record<string, unknown>
with an explicit whitelist type allowing only the documented keys. Change the
type of data on the CreateUserParams interface (symbol: CreateUserParams.data)
to something like an object with optional properties role?: string,
app_metadata?: Record<string, unknown>, and user_metadata?: Record<string,
unknown> so only those keys are accepted at compile time and other keys are
rejected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@eslint_temporary_suppressions.js`:
- Around line 527-647: The ESLint suppression entries in
eslint_temporary_suppressions.js currently disable multiple
'@typescript-eslint/no-unsafe-*' rules without tracking; for each suppression
object (the objects specifying files: [...] and rules: {...}) add explicit audit
metadata (e.g., an owner string and a removalTarget/date and optional
justification) so each block is temporary and discoverable—update every
suppression object in the file (including the ones that target production auth
flows like the objects covering auth.ts, refresh.ts, account.ts, user.ts,
cookies.ts, environment.ts and the test suppression objects) to include metadata
fields (owner and removalTarget) or add a single-line comment above the object
with that same metadata and a link to the tracking ticket; ensure the metadata
format is consistent across all blocks so automated tooling can parse it.

In `@packages/identity/prod/package.json`:
- Line 57: Update the package.json "directory" metadata value to point to the
actual package location; replace the current "directory" value
("packages/identity") with the correct path "packages/identity/prod" so the
package metadata matches the repository layout and tooling that relies on the
"directory" field.

In `@packages/identity/prod/src/events.ts`:
- Around line 56-66: The LOGIN handler can emit a null user when
getGoTrueClient()?.currentUser() hasn't refreshed; change the storage listener
so that when event.key === GOTRUE_STORAGE_KEY and event.newValue is truthy you
parse event.newValue (JSON.parse) to recover the user payload and pass that
through toUser before calling emitAuthEvent(AUTH_EVENTS.LOGIN, ...); if parsing
fails or event.newValue is missing, fall back to calling
getGoTrueClient().currentUser() and only emit LOGIN if that returns a non-null
value; wrap parsing in try/catch and on parse error do not emit LOGIN (or fall
back to LOGOUT) to avoid sending a null user.

In `@packages/identity/prod/src/fetch.ts`:
- Around line 19-33: The code currently ignores any incoming options.signal by
always passing controller.signal to fetch; preserve caller cancellation by
wiring the incoming signal to the local timeout controller: create the timeout
AbortController as before, but if options?.signal exists, if it's already
aborted call controller.abort(), otherwise add an event listener on
options.signal that calls controller.abort() so an upstream abort cancels the
fetch; pass controller.signal to fetch (do NOT call abort on the caller's
signal), and in the finally block remove the event listener and clearTimeout to
avoid leaks (referencing controller, timer, and options.signal in the fetch
call).

In `@packages/identity/prod/src/nextjs.ts`:
- Around line 40-42: Replace the overly-broad `'digest' in e` check with an
exact equality check for the documented Next.js bailout digest: ensure the
condition reads something like `e instanceof Error && (typeof (e as any).digest
=== 'string' && (e as any).digest === 'DYNAMIC_SERVER_USAGE' ||
/bail\s*out.*prerende/i.test(e.message))` so only errors whose digest equals
'DYNAMIC_SERVER_USAGE' (or whose message matches the existing regex) are
re-thrown; locate and update the existing `if (e instanceof Error && ('digest'
in e || /bail\s*out.*prerende/i.test(e.message))) { throw e }` block
accordingly.

In `@packages/identity/prod/src/types.ts`:
- Around line 21-23: The AppMetadata interface currently requires provider but
runtime/user-claim flows can omit it; change the AppMetadata definition so
provider is optional (make provider?: AuthProvider in the AppMetadata interface
in types.ts) and then update any call sites that assume a guaranteed provider to
handle the undefined case instead of using unsafe casts/suppressions.

In `@packages/identity/prod/test/config.browser.test.ts`:
- Around line 14-18: The test currently only asserts the path segment of
config.url, which allows relative URLs; update the assertion in the test for
getIdentityConfig to verify the full origin-derived URL by checking that
config.url either equals `${window.location.origin}/.netlify/identity` or at
minimum startsWith(window.location.origin) and includes '/.netlify/identity'
(use getIdentityConfig and config.url in your assertion).

---

Nitpick comments:
In `@packages/identity/prod/src/admin.ts`:
- Around line 51-74: The adminFetch function always adds 'Content-Type:
application/json' which is unnecessary for requests without a body (e.g.,
deleteUser). Update adminFetch to only set the Content-Type header when the
request has a body (e.g., when options.body is present) or when options.method
is not 'DELETE' (check options.method case-insensitively); keep the other
headers and Authorization behavior unchanged so existing calls still work.

In `@packages/identity/prod/src/auth.ts`:
- Around line 387-418: Replace the bare fetch call in handleEmailChangeCallback
with the project's fetchWithTimeout helper to match other auth requests and
avoid hanging; call fetchWithTimeout(`${identityUrl}/user`, { method: 'PUT',
headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${jwt}` },
body: JSON.stringify({ email_change_token: emailChangeToken }) }) using the same
timeout/opts pattern used elsewhere, preserve the current response/error
handling (checking emailChangeRes.ok, parsing json into
GoTrueErrorBody/UserData, throwing AuthError with status), and ensure
emitAuthEvent(AUTH_EVENTS.USER_UPDATED, user), clearHash(), and the returned {
type: 'email_change', user } remain unchanged.

In `@packages/identity/prod/src/cookies.ts`:
- Around line 19-41: Document the intentional choice to set both NF_JWT_COOKIE
and NF_REFRESH_COOKIE with httpOnly: false in the public API docs/README and in
any consumer-facing guidance: mention that setAuthCookies uses cookies.set to
expose nf_refresh to document.cookie for browser-side hydration
(backgroundHydrate/hydrateSession), explain the XSS risk of storing a refresh
token accessible to JavaScript, and add recommended mitigations (e.g., CSP,
input sanitization, dependency hygiene, automated scanning) and a clear note
that this trade-off is deliberate so integrators are aware when using
setAuthCookies and NF_REFRESH_COOKIE.

In `@packages/identity/prod/src/environment.ts`:
- Around line 20-37: Refactor discoverApiUrl to remove duplicated server-side
URL fallbacks by delegating server resolution to getIdentityContext: keep the
early cachedApiUrl and browser branch (isBrowser + window.location.origin +
IDENTITY_PATH) in discoverApiUrl, but for the non-browser path call
getIdentityContext() and use identityContext?.url (or construct with
IDENTITY_PATH from the returned context when present); remove the duplicated
Netlify/process.env URL checks from discoverApiUrl and ensure getIdentityContext
continues to handle globalThis.Netlify and process.env.URL logic so cachedApiUrl
and functions discoverApiUrl, getIdentityContext, IDENTITY_PATH, isBrowser, and
cachedApiUrl remain consistent.

In `@packages/identity/prod/src/refresh.ts`:
- Around line 153-159: The current identity URL assignment can throw if
globalThis.Netlify.context.url is malformed; wrap the URL construction in a
try/catch around the new URL(IDENTITY_PATH, globalThis.Netlify.context.url) so
that any URIError or other exception is caught and rethrown as an AuthError with
a clear message (include the original error message for debugging) before
assigning identityUrl; update the logic around getIdentityContext(),
IDENTITY_PATH and identityUrl to use this safe construction and ensure the final
thrown error remains an AuthError when URL construction fails.

In `@packages/identity/prod/src/types.ts`:
- Around line 145-150: CreateUserParams.data is too permissive; replace the
Record<string, unknown> with an explicit whitelist type allowing only the
documented keys. Change the type of data on the CreateUserParams interface
(symbol: CreateUserParams.data) to something like an object with optional
properties role?: string, app_metadata?: Record<string, unknown>, and
user_metadata?: Record<string, unknown> so only those keys are accepted at
compile time and other keys are rejected.

In `@packages/identity/prod/test/account.browser.test.ts`:
- Around line 224-236: Remove the redundant cookie reset at the end of the test:
the test already sets document.cookie to '' before calling updateUser and
clearBrowserAuthCookies() is executed in afterEach, so delete the second
Object.defineProperty(document, 'cookie', { writable: true, value: '' }) near
the end of the 'throws AuthError when no user is logged in' test (references:
updateUser, mockCurrentUser, document.cookie, clearBrowserAuthCookies,
afterEach).

In `@packages/identity/prod/test/admin.browser.test.ts`:
- Around line 24-71: Extract the repeated import/catch/assert pattern into a
small helper in the test file (e.g. async function assertAdminAuthError(run: ()
=> Promise<unknown>)) and use it for each case instead of duplicating code; the
helper should import AuthError once, call the provided admin method
(admin.listUsers, admin.getUser, admin.createUser, admin.updateUser,
admin.deleteUser) capturing any thrown value, assert it is an instance of
AuthError and that its message contains 'server-only'. Replace each test body to
call this helper with a lambda invoking the corresponding admin.* method to
remove the boilerplate.

In `@packages/identity/prod/test/config.test.ts`:
- Around line 77-81: The test calls getSettings() twice which can make it flaky;
call getSettings() once and capture its rejection (either via await
expect(getSettings()).rejects.toThrow(MissingIdentityError) combined in one
assertion or by awaiting getSettings() in a try/catch and asserting the caught
error is an instance of MissingIdentityError and its message contains 'Netlify
Identity is not available'); update the spec that references getSettings and
MissingIdentityError to use the single captured error for both checks.
🪄 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: 1bda4e68-4cad-4054-a19c-312664346a21

📥 Commits

Reviewing files that changed from the base of the PR and between e04b68a and efa17c8.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (43)
  • .github/CODEOWNERS
  • .github/workflows/release-please.yaml
  • .release-please-manifest.json
  • eslint_temporary_suppressions.js
  • package.json
  • packages/identity/.gitignore
  • packages/identity/prod/.gitignore
  • packages/identity/prod/package.json
  • packages/identity/prod/src/account.ts
  • packages/identity/prod/src/admin.ts
  • packages/identity/prod/src/auth.ts
  • packages/identity/prod/src/config.ts
  • packages/identity/prod/src/cookies.ts
  • packages/identity/prod/src/environment.ts
  • packages/identity/prod/src/errors.ts
  • packages/identity/prod/src/events.ts
  • packages/identity/prod/src/fetch.ts
  • packages/identity/prod/src/globals.d.ts
  • packages/identity/prod/src/main.ts
  • packages/identity/prod/src/nextjs.ts
  • packages/identity/prod/src/refresh.ts
  • packages/identity/prod/src/types.ts
  • packages/identity/prod/src/user.ts
  • packages/identity/prod/test/account.browser.test.ts
  • packages/identity/prod/test/admin.browser.test.ts
  • packages/identity/prod/test/admin.server.test.ts
  • packages/identity/prod/test/auth.browser.test.ts
  • packages/identity/prod/test/auth.server.test.ts
  • packages/identity/prod/test/config.browser.test.ts
  • packages/identity/prod/test/config.test.ts
  • packages/identity/prod/test/errors.test.ts
  • packages/identity/prod/test/fetch.test.ts
  • packages/identity/prod/test/fixtures.ts
  • packages/identity/prod/test/nextjs.test.ts
  • packages/identity/prod/test/refresh.browser.test.ts
  • packages/identity/prod/test/refresh.server.test.ts
  • packages/identity/prod/test/stale-session.browser.test.ts
  • packages/identity/prod/test/user.browser.test.ts
  • packages/identity/prod/test/user.test.ts
  • packages/identity/prod/tsconfig.json
  • packages/identity/prod/tsup.config.ts
  • packages/identity/prod/vitest.config.ts
  • release-please-config.json

Comment thread eslint_temporary_suppressions.js
Comment thread packages/identity/prod/package.json Outdated
Comment thread packages/identity/prod/src/events.ts
Comment thread packages/identity/prod/src/fetch.ts
Comment thread packages/identity/prod/src/nextjs.ts
Comment thread packages/identity/prod/src/types.ts
Comment thread packages/identity/prod/test/config.browser.test.ts
@khendrikse khendrikse force-pushed the EX-1826/add-identity-package branch from d24832a to 620be93 Compare April 30, 2026 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants