Adds app and pages router entrypoints#399
Conversation
|
Important Review skippedThis PR was authored by the user configured for CodeRabbit reviews. CodeRabbit does not review PRs authored by this user. It's recommended to use a dedicated user account to post CodeRabbit review feedback. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughAdds explicit app/pages router import entry points, client re-exports for React hooks, consolidated server-side re-exports, new server storage and helper utilities with safe wrappers, a server-user extractor, Vite lib entries for new entrypoints, and tests validating entrypoints and server helpers. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Server code
participant Entrypoint as app/pages server entry
participant Factory as create*ServerHelpers()
participant Safe as safe wrapper
participant Utils as `@kinde/js-utils`
participant Store as Session/MemoryStorage
Dev->>Entrypoint: import { createAppServerHelpers / createPagesServerHelpers }
Entrypoint->>Factory: call factory (req?, res?)
Factory->>Safe: wrap helpers calls (getClaim/getFlag/getAccessToken...)
Dev->>Factory: helpers.getAccessToken()/getFlag()/getClaim()
Safe->>Utils: getRawToken/getDecodedToken/getFlag/getEntitlements...
Utils->>Store: read tokens/session items
alt success
Utils-->>Safe: values
Safe-->>Dev: normalized value
else error
Utils-->>Safe: throws
Safe-->>Dev: default/fallback
end
sequenceDiagram
autonumber
actor Dev as Server code
participant getServerUser as getServerUser()
participant StorageFlow as withServerStorage / hydrateServerStorage
participant Utils as `@kinde/js-utils`
participant Store as ID token
Dev->>getServerUser: await getServerUser(req?, res?)
getServerUser->>StorageFlow: hydrate & access idToken
StorageFlow->>Utils: getRawToken(idToken) -> getDecodedToken
Utils->>Store: read token
alt decoded present
getServerUser-->>Dev: { id, email, given_name, family_name, picture }
else missing/invalid
getServerUser-->>Dev: null
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
dtoxvanilla1991
force-pushed
the
feat/app-pages-routes-separation
branch
from
915a053 to
4537491
Compare
dtoxvanilla1991
force-pushed
the
feat/app-pages-routes-separation
branch
from
5e69a05 to
262161d
Compare
…and exports - Removed legacy session wrappers and consolidated session handling into new `createAppRouterSession` and `createPagesRouterSession` functions. - Updated entry points for App Router and Pages Router to utilize `@kinde-oss/kinde-auth-react` for client exports. - Simplified server exports by removing unnecessary components and legacy code. - Deleted outdated tests related to the old session structure and added new tests to verify the new session helpers. - Ensured backward compatibility by retaining legacy session exports while promoting new session management practices.
…nce clarity and maintainability
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
src/app/server/index.ts (1)
1-5: Consider aligning export order with pages/server for consistency.The export order differs slightly from
src/pages/server/index.ts:
- App: withAuth → createKindeManagementAPIClient → handleAuth → protectApi → protectPage
- Pages: withAuth → protectApi → protectPage → handleAuth → createKindeManagementAPIClient
While not a functional issue, consistent ordering improves maintainability.
Apply this diff to align with the pages variant:
export { withAuth } from "../../authMiddleware/authMiddleware"; -export { createKindeManagementAPIClient } from "../../api-client"; -export { default as handleAuth } from "../../handlers/auth"; export { protectApi, protectPage } from "../../handlers/protect"; +export { default as handleAuth } from "../../handlers/auth"; +export { createKindeManagementAPIClient } from "../../api-client"; export * from "../../types";src/server/createServerHelpers.ts (3)
40-40: Return typePromise<T | any>defeats generic type checking.The return type
Promise<T | any>makes the generic parameterTmeaningless becauseanyabsorbs all types. This undermines TypeScript's type safety.Consider using
Promise<T>and ensuring the fallback matches type T, or use a union type that preserves type information:-const safe = async <T>(fn: () => Promise<T>, fallback: any = null): Promise<T | any> => { +const safe = async <T>(fn: () => Promise<T>, fallback: T): Promise<T> => { try { return await fn(); } catch { return fallback; } };Note: This would require updating all call sites to provide properly typed fallback values.
60-74: Verify flag null handling before type checks.Lines 62, 67, and 72 perform
typeofchecks without first verifying ifvis null. While the safe wrapper returns the default on errors, explicitly checking for null would make the intent clearer.Apply this diff to add explicit null checks:
getBooleanFlag: (code, defaultValue) => safe(async () => { const v: any = await jsGetFlag(code); - return typeof v === "boolean" ? v : defaultValue; + return v != null && typeof v === "boolean" ? v : defaultValue; }, defaultValue), getIntegerFlag: (code, defaultValue) => safe(async () => { const v: any = await jsGetFlag(code); - return typeof v === "number" ? v : defaultValue; + return v != null && typeof v === "number" ? v : defaultValue; }, defaultValue), getStringFlag: (code, defaultValue) => safe(async () => { const v: any = await jsGetFlag(code); - return typeof v === "string" ? v : defaultValue; + return v != null && typeof v === "string" ? v : defaultValue; }, defaultValue),
75-81: Remove doubleas anycasts onjsGetClaim
These assertions defeat TypeScript safety. Confirm the true signature ofjsGetClaimin your package’s typings and, if it’s correctly typed, drop them or replace them with precise types.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
package.jsonis excluded by!**/*.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/*.yaml
📒 Files selected for processing (11)
readme.md(1 hunks)src/app/index.ts(1 hunks)src/app/server/index.ts(1 hunks)src/pages/index.ts(1 hunks)src/pages/server/index.ts(1 hunks)src/server/createServerHelpers.ts(1 hunks)src/server/getServerUser.ts(1 hunks)tests/entrypoints/app-pages-entrypoints.test.ts(1 hunks)tests/server/app-server-helpers.test.ts(1 hunks)tests/server/pages-server-helpers.test.ts(1 hunks)vite.config.mts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/server/getServerUser.ts (2)
src/app/server/index.ts (1)
getServerUser(9-9)src/pages/server/index.ts (1)
getServerUser(9-9)
tests/server/pages-server-helpers.test.ts (2)
src/server/createServerHelpers.ts (1)
createPagesServerHelpers(103-103)src/pages/server/index.ts (1)
createPagesServerHelpers(8-8)
tests/server/app-server-helpers.test.ts (2)
src/server/createServerHelpers.ts (1)
createAppServerHelpers(97-97)src/server/getServerUser.ts (1)
getServerUser(15-29)
src/server/createServerHelpers.ts (2)
src/app/server/index.ts (1)
createAppServerHelpers(8-8)src/pages/server/index.ts (1)
createPagesServerHelpers(8-8)
🪛 markdownlint-cli2 (0.18.1)
readme.md
41-41: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (9)
src/app/index.ts (1)
1-2: LGTM!The re-exports are clean and provide a clear entry point for App Router users. This mirrors the pattern used in
src/pages/index.tsand maintains consistency across router-specific entry points.vite.config.mts (1)
23-26: LGTM!The new entry points are correctly configured and align with the new router-specific modules added in this PR. The build configuration properly extends the existing setup without modifying any existing entries.
src/pages/index.ts (1)
1-2: LGTM!The re-exports provide a clear entry point for Pages Router users and maintain consistency with the App Router entry point in
src/app/index.ts.tests/entrypoints/app-pages-entrypoints.test.ts (1)
3-19: LGTM!These tests effectively validate the new entry points by checking both positive (new helpers exist) and negative (legacy wrappers are absent) conditions. This ensures the public API surface is correct and prevents accidental exposure of deprecated functions.
tests/server/app-server-helpers.test.ts (1)
27-66: LGTM!Comprehensive test coverage that validates the helper API surface, fallback behaviors, user object shape, and claim normalization. This serves as a good template for the pages server helpers tests.
src/pages/server/index.ts (1)
1-9: LGTM! Clean entry point for Pages Router server utilities.The re-exports consolidate the Pages Router server-side API effectively, providing a clear import path (
@kinde-oss/kinde-auth-nextjs/pages/server) for developers.src/app/server/index.ts (1)
7-9: LGTM! Proper App Router server helpers export.The createAppServerHelpers factory is correctly exposed for App Router usage.
src/server/createServerHelpers.ts (2)
93-103: LGTM! Factory functions correctly implement router-specific helpers.The unused
_reqand_resparameters increatePagesServerHelpersare appropriately prefixed with underscores and documented. This design choice supports future evolution while maintaining API compatibility.
16-37: Consider improving type safety in the public interfaceI couldn’t locate type definitions in
@kinde/js-utils; please verify if it exports specific types for tokens, permissions, flags, etc., and replaceanyaccordingly.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/server/createServerHelpers.ts (1)
90-91: Consider simplifying the double fallback.The
isAuthenticatedimplementation has both an inline|| falseand thesafewrapper's fallback parameter set tofalse. While harmless, this creates redundancy.You could simplify to just rely on the safe wrapper's fallback:
- isAuthenticated: () => - safe(async () => (await jsIsAuthenticated()) || false, false), + isAuthenticated: () => + safe(async () => Boolean(await jsIsAuthenticated()), false),Or simply:
- isAuthenticated: () => - safe(async () => (await jsIsAuthenticated()) || false, false), + isAuthenticated: () => safe(() => jsIsAuthenticated(), false),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
package.jsonis excluded by!**/*.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/*.yaml
📒 Files selected for processing (4)
src/server/createServerHelpers.ts(1 hunks)tests/entrypoints/app-pages-entrypoints.test.ts(1 hunks)tests/server/app-server-helpers.test.ts(1 hunks)tests/server/pages-server-helpers.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/server/pages-server-helpers.test.ts
🧰 Additional context used
🧬 Code graph analysis (2)
src/server/createServerHelpers.ts (3)
src/utils/removeUndefined.ts (1)
T(1-5)src/app/server/index.ts (1)
createAppServerHelpers(8-8)src/pages/server/index.ts (1)
createPagesServerHelpers(8-8)
tests/server/app-server-helpers.test.ts (2)
src/server/createServerHelpers.ts (1)
createAppServerHelpers(100-101)src/server/getServerUser.ts (1)
getServerUser(15-29)
🔇 Additional comments (4)
tests/entrypoints/app-pages-entrypoints.test.ts (1)
1-19: LGTM!The entry point tests effectively verify that the new router-specific helpers are exported while confirming the absence of legacy wrappers. The use of dynamic imports to test actual entry points and safe property checks with
Object.prototype.hasOwnProperty.callare both appropriate.tests/server/app-server-helpers.test.ts (1)
1-66: LGTM!The test suite provides comprehensive coverage of the app server helpers, including:
- API surface verification with all expected function keys
- Fallback behavior for typed flag getters
- User extraction via getServerUser
- Token key normalization in getClaim
The mock setup is thorough and the test cases validate both the shape and behavior of the helpers.
src/server/createServerHelpers.ts (2)
16-37: LGTM!The
KindeServerHelpersinterface is well-designed with clear method signatures and appropriate use of generics. The type definitions provide good developer experience and type safety for consumers of the server helpers.
96-110: LGTM!The factory functions are clean and well-documented. The unused
_reqand_resparameters increatePagesServerHelpersare appropriately prefixed and documented for future extensibility, maintaining API compatibility with legacy signatures.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/server/createServerHelpers.ts(1 hunks)src/server/getServerUser.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/server/createServerHelpers.ts (2)
src/app/server/index.ts (1)
createAppServerHelpers(8-8)src/pages/server/index.ts (1)
createPagesServerHelpers(8-8)
🪛 GitHub Actions: Build and test
src/server/createServerHelpers.ts
[warning] 1-1: Prettier formatting issues found. Run 'prettier --write' to fix.
src/server/getServerUser.ts
[warning] 1-1: Prettier formatting issues found. Run 'prettier --write' to fix.
…ctions to utilize it
…nd TypeScript configuration
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/server/getServerUser.ts (1)
31-34: Critical: Missing sub validation guard.The type assertion on line 31 assumes
getDecodedTokenreturns a fully-formedDecodedIdToken, but it may returnnullor a token wheresubisundefinedor empty. While line 32 handlesnull, it doesn't validateidTok.subbefore assigning it toServerUser.idon line 34. This violates theServerUsercontract whereidmust be astring, notundefined.Apply this diff to add the sub validation guard:
return await withServerStorage(req, res, async () => { - const idTok: DecodedIdToken = await getDecodedToken(StorageKeys.idToken); + const idTok = await getDecodedToken(StorageKeys.idToken); if (!idTok) return null; + + const sub = idTok.sub; + if (!sub || typeof sub !== 'string') { + if (process.env.NODE_ENV !== "production") { + console.warn("[getServerUser] ID token missing required 'sub' claim"); + } + return null; + } + return { - id: idTok.sub, + id: sub, email: idTok.email, given_name: idTok.given_name, family_name: idTok.family_name, picture: idTok.picture, }; });
🧹 Nitpick comments (3)
src/server/serverStorage.ts (2)
13-13: Consider stronger typing for request/response parameters.The
reqandresparameters are typed asany, which loses type safety. Consider defining interfaces or using Next.js types (e.g.,NextRequest,NextResponsefor App Router orNextApiRequest,NextApiResponsefor Pages Router).Example for Pages Router:
import type { NextApiRequest, NextApiResponse } from 'next'; export const hydrateServerStorage = async ( req?: NextApiRequest, res?: NextApiResponse ) => {
61-63: Apply the same typing improvement for consistency.The
reqandresparameters should use the same stronger types recommended forhydrateServerStorageto maintain consistency across the API surface.+import type { NextApiRequest, NextApiResponse } from 'next'; + export const withServerStorage = async <T>( - req: any, - res: any, + req: NextApiRequest, + res: NextApiResponse, fn: () => Promise<T>, ): Promise<T> => {src/server/getServerUser.ts (1)
25-27: Consider stronger typing for request/response parameters.Similar to
serverStorage.ts, thereqandresparameters are typed asany. For consistency and type safety across the server API surface, define proper interfaces or use Next.js-specific types.+import type { NextApiRequest, NextApiResponse } from 'next'; + export const getServerUser = async ( - req?: any, - res?: any, + req?: NextApiRequest, + res?: NextApiResponse, ): Promise<ServerUser | null> => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
playground/tsconfig.jsonis excluded by!**/*.json
📒 Files selected for processing (3)
src/server/createServerHelpers.ts(1 hunks)src/server/getServerUser.ts(1 hunks)src/server/serverStorage.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/server/createServerHelpers.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/server/getServerUser.ts (1)
src/server/serverStorage.ts (1)
withServerStorage(61-72)
🔇 Additional comments (4)
src/server/serverStorage.ts (2)
66-71: The wrapper pattern ensures proper cleanup.The try-finally block guarantees that cleanup executes even if the provided function throws, which is the correct approach for resource management.
51-57: <!-- [scratchpad]
[task verify_cleanup_order 3/3 complete]
Gathered sufficient information to assess review concern.[observations]
- @kinde/js-utils version: ^0.24.0 (external library)
- Both clearActiveStorage and MemoryStorage from @kinde/js-utils
- storage variable captured in closure (line 14)
- cleanup handler returns async function (lines 51-57)
- Pattern consistent in getEntitlements.ts (lines 40-41)
- Comment states: "Returns a cleanup handler that clears the in-memory storage after use"
- Finally block ensures destroySession runs even if clearActiveStorage throws
[analysis]
- Cannot inspect external library source code to verify destroySession implementation
- However, closure captures storage instance; clearActiveStorage only clears global active reference
- Pattern appears intentional: clear active global state, then destroy instance
- Multiple files follow same pattern, suggesting design intent
- Finally block ensures destroySession executes regardless of exceptions
[conclusion]
Evidence suggests cleanup order is intentional. The storage instance remains usable after clearActiveStorage clears the active reference. Without external library source, cannot definitively confirm destroySession doesn't access getActiveStorage(), but pattern consistency indicates it likely doesn't require active storage.[done]
Review concern cannot be fully resolved due to external library constraint. Recommend manual verification or checking library documentation.
Cleanup order appears intentional; external library constraint prevents full verification.
The
storagevariable is captured in the closure and remains available afterclearActiveStorage()clears the global active reference. The pattern is consistent across the codebase (also insrc/session/getEntitlements.tslines 40-41) and uses a finally block to ensuredestroySession()executes. However, I cannot inspect the@kinde/js-utils(^0.24.0)destroySession()implementation to definitively confirm it doesn't access the active storage state.To fully verify: check the @kinde/js-utils package documentation or source for whether
destroySession()requires access togetActiveStorage().src/server/getServerUser.ts (2)
4-19: Well-structured interface definitions.The separation between
ServerUser(public contract) andDecodedIdToken(internal token shape) provides clear boundaries and type safety. The requiredid/subfields correctly enforce the authentication identity requirement.
41-46: Error logging approach is reasonable.Logging errors in non-production environments while returning
nullin all cases provides a balance between debuggability and graceful failure. The catch-all approach is appropriate here since any decoding failure should result in treating the user as unauthenticated.
…improve readability
…for server helper functions
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
tests/server/pages-server-helpers.test.ts (1)
23-28: Expand test coverage to match app-server-helpers.test.ts.The test coverage here remains minimal. Since
createPagesServerHelpersandcreateAppServerHelpersshould provide equivalent functionality, the test suite should validate all helper keys, flag fallback behavior,getServerUserbehavior, and claim normalization—mirroring the coverage intests/server/app-server-helpers.test.ts.Consider applying the diff suggested in the previous review to add comprehensive test coverage for all helper functions.
🧹 Nitpick comments (1)
tests/server/app-server-helpers.test.ts (1)
20-21: Consider instantiating helpers in beforeEach for test isolation.The helpers object is created once at describe-block scope, meaning all tests share the same instance. If the helpers maintain any internal state or cache references to mock implementations, this could introduce test interdependencies.
Consider moving the instantiation into a beforeEach block to ensure each test gets a fresh helpers instance:
describe("createAppServerHelpers", () => { - const helpers = createAppServerHelpers(); + let helpers: ReturnType<typeof createAppServerHelpers>; const keys = [ "getAccessToken", // ... rest of keys ]; + beforeEach(() => { + helpers = createAppServerHelpers(); + }); it("exposes expected function keys", () => {Note: This may not be strictly necessary if the helpers are stateless wrappers, but it improves test isolation guarantees.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
tests/server/app-server-helpers.test.ts(1 hunks)tests/server/pages-server-helpers.test.ts(1 hunks)tests/server/setup-server-helper-mocks.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tests/server/app-server-helpers.test.ts (3)
tests/server/setup-server-helper-mocks.ts (3)
resetServerHelperMocks(74-79)jsUtilsMockFns(15-27)storageKeys(3-7)src/server/createServerHelpers.ts (1)
createAppServerHelpers(136-137)src/server/getServerUser.ts (1)
getServerUser(25-47)
tests/server/pages-server-helpers.test.ts (2)
tests/server/setup-server-helper-mocks.ts (2)
resetServerHelperMocks(74-79)jsUtilsMockFns(15-27)src/server/createServerHelpers.ts (1)
createPagesServerHelpers(143-146)
🔇 Additional comments (5)
tests/server/app-server-helpers.test.ts (1)
40-58: LGTM! Comprehensive test coverage.The test suite thoroughly validates:
- All expected helper function keys exist and are callable
- Boolean flag fallback behavior with missing flags
- getServerUser returns properly shaped user objects
- getClaim correctly normalizes tokenKey variants (accessToken/idToken)
This provides good coverage of the core helper functionality.
tests/server/setup-server-helper-mocks.ts (4)
3-27: LGTM! Well-structured mock fixtures.The exported constants and mock functions provide a clean, centralized test fixture:
storageKeys: consistent key names for token storagestorageTokenValues: deterministic mock token stringsjsUtilsMockFns: comprehensive set of mocked js-utils functionsThis structure promotes test consistency and makes it easy for tests to configure specific mock behaviors.
84-102: LGTM! Clean in-memory storage mock.The
FakeMemoryStorageimplementation provides deterministic, isolated storage for tests using a simpleMap-based approach. All required methods are properly implemented as async functions matching the expected storage interface.Minor note: The
String(key)coercion on lines 88, 92, and 96 is defensive but likely unnecessary sincekeyis already typed asstring. However, it's harmless and could prevent issues if type constraints change in the future.
46-81: LGTM! Robust mock configuration and reset utilities.The mock management is well-designed:
setDefaultImplementations: configures sensible default return values for all mocked utilitiesresetServerHelperMocks: provides clean reset between tests by clearing all mocks and reapplying defaults- Module-level initialization (line 81) ensures mocks are ready immediately
This pattern ensures predictable mock behavior and proper test isolation.
83-115: LGTM! Properly structured module mocks.The
vi.mockdeclarations are correctly placed at module scope and provide comprehensive mock implementations:
@kinde/js-utils: exportsStorageKeys,MemoryStorage(asFakeMemoryStorage), storage control functions, and all utility mocks fromjsUtilsMockFnssessionManager: exposes the configurablesessionManagerMockThis mocking strategy allows tests to control all external dependencies while maintaining realistic interfaces.
…on with additional checks
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/server/getServerUser.ts (1)
43-45: Use a strict development-only log guard in server utilities.Please switch this condition to
process.env.NODE_ENV === "development"so test/other non-prod environments don’t get forced SDK error noise.Proposed change
- if (process.env.NODE_ENV !== "production") { + if (process.env.NODE_ENV === "development") { console.error("[getServerUser] Failed to decode ID token:", err); }Based on learnings: In the SDK’s server-side utilities, keep logging dev-only and guard logs with
process.env.NODE_ENV === "development".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/server/getServerUser.ts` around lines 43 - 45, In getServerUser update the dev-only log guard: replace the current condition that checks process.env.NODE_ENV !== "production" with process.env.NODE_ENV === "development" so the console.error("[getServerUser] Failed to decode ID token:", err) runs only in development; keep the same error variable (err) and logging call but narrow the environment check to development-only.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/server/pages-server-helpers.test.ts`:
- Around line 8-10: The test is importing createPagesServerHelpers (and
getServerUser) directly from internal server modules which bypasses the new
Pages router public entrypoint; update the imports in this test to pull
createPagesServerHelpers (and any pages-specific helpers like getServerUser if a
pages export exists) from the Pages-specific public entrypoint instead of
"../../src/server/createServerHelpers" so the test exercises the exported
entrypoint layer and will catch export-map/entrypoint regressions.
---
Nitpick comments:
In `@src/server/getServerUser.ts`:
- Around line 43-45: In getServerUser update the dev-only log guard: replace the
current condition that checks process.env.NODE_ENV !== "production" with
process.env.NODE_ENV === "development" so the console.error("[getServerUser]
Failed to decode ID token:", err) runs only in development; keep the same error
variable (err) and logging call but narrow the environment check to
development-only.
🪄 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: 5aaf27a2-2e26-43df-8e29-28d808aaae38
⛔ Files ignored due to path filters (2)
package.jsonis excluded by!**/*.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/*.yaml
📒 Files selected for processing (5)
src/server/getServerUser.tstests/server/pages-server-helpers.test.tstests/server/setup-server-helper-mocks.tsvite.config.mtsvitest.config.ts
✅ Files skipped from review due to trivial changes (1)
- vitest.config.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- vite.config.mts
| import { createPagesServerHelpers } from "../../src/server/createServerHelpers"; | ||
| import { getServerUser } from "../../src/server/getServerUser"; | ||
|
|
There was a problem hiding this comment.
Test is bypassing the Pages router entrypoint being introduced.
This suite should import from the pages-specific public entrypoint, otherwise export-map/entrypoint regressions won’t be detected.
Proposed change
-import { createPagesServerHelpers } from "../../src/server/createServerHelpers";
-import { getServerUser } from "../../src/server/getServerUser";
+import { createPagesServerHelpers, getServerUser } from "../../src/pages/server/index";📝 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.
| import { createPagesServerHelpers } from "../../src/server/createServerHelpers"; | |
| import { getServerUser } from "../../src/server/getServerUser"; | |
| import { createPagesServerHelpers, getServerUser } from "../../src/pages/server/index"; | |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/server/pages-server-helpers.test.ts` around lines 8 - 10, The test is
importing createPagesServerHelpers (and getServerUser) directly from internal
server modules which bypasses the new Pages router public entrypoint; update the
imports in this test to pull createPagesServerHelpers (and any pages-specific
helpers like getServerUser if a pages export exists) from the Pages-specific
public entrypoint instead of "../../src/server/createServerHelpers" so the test
exercises the exported entrypoint layer and will catch export-map/entrypoint
regressions.
1 participant
Adds new entrypoints for the app and pages routers.
These entrypoints mirror the existing root exports to maintain current functionality and provide router-specific access points.
This allows developers to import from a specific path and clarify the intended router usage.
Explain your changes
Suppose there is a related issue with enough detail for a reviewer to understand your changes fully. In that case, you can omit an explanation and instead include either “Fixes #XX” or “Updates #XX” where “XX” is the issue number.
Checklist
🛟 If you need help, consider asking for advice over in the Kinde community.