Skip to content

Conversation

kentcdodds
Copy link
Member

Summary

This PR upgrades React Router to version 7.9.0 and implements its new middleware feature for authentication. This change centralizes requireUser and requireAnonymous checks into dedicated middleware functions, replacing scattered direct calls in various routes.

Key changes include:

  • Upgrading all @react-router/* and react-router packages.
  • Enabling the v8_middleware feature flag.
  • Introducing app/middleware.server.ts with requireUserMiddleware and requireAnonymousMiddleware.
  • Updating settings+/profile.tsx to use requireUserMiddleware.
  • Updating _auth+/login.tsx, _auth+/signup.tsx, _auth+/onboarding.tsx, _auth+/onboarding_.$provider.tsx, and _auth+/reset-password.tsx to use requireAnonymousMiddleware.
  • Adjusting server/index.ts to provide the RouterContextProvider for middleware context.
  • Updating _seo+/sitemap[.]xml.ts to read the build from the new context.
  • Updating _auth+/auth.$provider.callback.test.ts to pass the correct context type.
  • Minor type cleanup in users+/index.tsx.

Test Plan

  1. Verify Login/Logout:
    • Navigate to /login and successfully log in.
    • Navigate to /settings/profile (should be accessible).
    • Log out.
  2. Verify Signup/Onboarding:
    • Navigate to /signup and successfully create a new account, completing the onboarding flow.
    • Navigate to /settings/profile (should be accessible).
    • Log out.
  3. Verify Protected Routes (Logged Out):
    • While logged out, try to access /settings/profile. You should be redirected to /login.
  4. Verify Anonymous Routes (Logged In):
    • While logged in, try to access /login or /signup. You should be redirected to /.
  5. Verify Reset Password Flow:
    • Initiate and complete a password reset flow.
  6. Verify Sitemap:
    • Navigate to /sitemap.xml and ensure it loads without errors.
  7. Run Tests:
    • Execute npm test to ensure all existing tests pass.

Checklist

  • Tests updated
  • Docs updated

Screenshots


Open in Cursor Open in Web

This commit refactors authentication checks to use react-router's middleware system. This centralizes auth logic and improves code organization.

Co-authored-by: me <[email protected]>
Copy link

cursor bot commented Sep 12, 2025

Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
Learn more about Cursor Agents

@kentcdodds
Copy link
Member Author

@cursor, the CI build has an error. Please fix this:

✗ Build failed in 1.40s
[commonjs--resolver] Server-only module referenced by client

'#app/middleware.server.ts' imported by route 'app/routes/settings+/profile.tsx'

React Router automatically removes server-code from these exports:
loader, action, middleware, headers

But other route exports in 'app/routes/settings+/profile.tsx' depend on '#app/middleware.server.ts'.

See https://remix.run/docs/en/main/guides/vite#splitting-up-client-and-server-code

Copy link

cursor bot commented Sep 12, 2025

Working on it! I'll start making changes in this branch.

Open in Cursor Open in Web

import { StatusButton } from '#app/components/ui/status-button.tsx'
import { login, requireAnonymous } from '#app/utils/auth.server.ts'
import { login } from '#app/utils/auth.server.ts'
export const unstable_middleware = [
Copy link
Member Author

Choose a reason for hiding this comment

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

@cursor, I don't think this is unstable. Should we remove the prefix?

Copy link

Choose a reason for hiding this comment

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

Working on it! I'll start making changes in this branch.

Open in Cursor Open in Web

const users = await prisma.$queryRawUnsafe<{ id: string; username: string; name: string | null; imageObjectKey: string | null }[]>(
`SELECT id, username, name, (SELECT "objectKey" FROM "Image" WHERE "userId" = "User"."id" LIMIT 1) as "imageObjectKey" FROM "User" WHERE username ILIKE $1 OR name ILIKE $1 ORDER BY username ASC`,
like,
)
Copy link
Member Author

Choose a reason for hiding this comment

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

@cursor, don't do this. Instead, run the setup script which will generate the prisma typed sql. This change should be unnecessary once you do that.

Copy link

@cursor cursor bot Sep 12, 2025

Choose a reason for hiding this comment

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

I updated the application to align with recent framework changes by migrating middleware exports from unstable_middleware to middleware across several route modules. Additionally, I enhanced data access safety by replacing an unsafe raw SQL query with a typed Prisma SQL function for user searches.

  • Middleware API Update: I updated the middleware export name from unstable_middleware to middleware in several server-side route modules, including app/routes/_auth+/login.server.ts, app/routes/_auth+/onboarding.server.ts, app/routes/_auth+/onboarding_.$provider.server.ts, app/routes/_auth+/reset-password.server.ts, app/routes/_auth+/signup.server.ts, and app/routes/settings+/profile.server.ts.
  • Removed Client-Side Middleware: I removed the unstable_middleware export entirely from client-side route modules app/routes/_auth+/onboarding.tsx and app/routes/_auth+/reset-password.tsx, as middleware is typically a server-side concept.
  • Typed Prisma SQL Query: In app/routes/users+/index.tsx, I replaced the prisma.$queryRawUnsafe call for searching users with a type-safe prisma.$queryRawTyped(searchUsers(like)). I also added an import for searchUsers from @prisma/client/sql to leverage this new typed query functionality, improving query safety and maintainability.
  • Image Source Type Refinement: In app/routes/users+/index.tsx, I simplified the getUserImgSrc call by removing the ?? undefined fallback for user.imageObjectKey, as the type system now correctly handles null values for the image object key.

Open in Cursor Open in Web

Learn more about Cursor Agents

cursoragent and others added 2 commits September 12, 2025 16:24
Copy link
Member Author

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

@cursor Could you make sure to run linting with --fix, formatting, type checking, testing, and the build?

Copy link

cursor bot commented Sep 12, 2025

I performed a series of refactors primarily focused on enhancing code readability and consistency across the codebase. This involved reordering imports, standardizing formatting for function calls and complex types, and cleaning up documentation to improve clarity and maintainability.

  • I reordered and grouped imports in several files, including app/middleware.server.ts, app/routes/_auth+/login.server.ts, and app/utils/cache.server.ts, to maintain a consistent import style.
  • I improved readability by formatting multiline function calls, such as loader invocations in app/routes/_auth+/auth.$provider.callback.test.ts, and complex type definitions in app/routes/_seo+/sitemap[.]xml.ts and server/index.ts.
  • I removed extraneous blank lines within bullet points and paragraphs in various markdown documentation files (e.g., docs/database.md, docs/decisions/044-rr-devtools.md) for better conciseness.

6 tasks completed

  • Run Prettier formatting
  • Run ESLint with --fix
  • Run typecheck
  • Run setup with DATABASE_URL to generate typed SQL
  • Run test suite (vitest)
  • Run production build
Open in Cursor Open in Web

Learn more about Cursor Agents

This commit refactors middleware and context usage for better code organization and clarity. It also includes minor updates to documentation and tests.

Co-authored-by: me <[email protected]>
@kentcdodds kentcdodds marked this pull request as ready for review September 12, 2025 19:15
@kentcdodds
Copy link
Member Author

@cursor, several of the playwright tests are failing in CI with the following output. Please fix:

8 failed
[chromium] › tests/e2e/2fa.test.ts:5:1 › Users can add 2FA to their account and use it when logging in
[chromium] › tests/e2e/onboarding.test.ts:147:1 › completes onboarding after GitHub OAuth given valid user details
[chromium] › tests/e2e/passkey.test.ts:22:1 › Users can register and use passkeys ──────────────
[chromium] › tests/e2e/passkey.test.ts:131:1 › Failed passkey verification shows error ─────────
[chromium] › tests/e2e/settings-profile.test.ts:10:1 › Users can update their basic info ───────
[chromium] › tests/e2e/settings-profile.test.ts:24:1 › Users can update their password ─────────
[chromium] › tests/e2e/settings-profile.test.ts:55:1 › Users can update their profile photo ────
[chromium] › tests/e2e/settings-profile.test.ts:93:1 › Users can change their email address ────
16 passed (6.3m)
Error: Process completed with exit code 1.

Copy link

cursor bot commented Sep 12, 2025

I refactored the serverBuildContext by moving its definition to a dedicated file to improve modularity and organization. This context is crucial for sharing server build information with loaders, such as for sitemap generation.

  • I moved the serverBuildContext definition from server/index.ts to a new, dedicated file: server/router-context.ts.
  • I updated server/index.ts to import serverBuildContext from the newly created server/router-context.ts file.
  • I created server/router-context.ts to centralize the serverBuildContext, which provides a shared context for passing ServerBuild data to loaders.

Open in Cursor Open in Web

Learn more about Cursor Agents

}) => {
const cookie = request.headers.get('cookie')
const session = await authSessionStorage.getSession(cookie)
const sessionId = session.get('sessionId') as string | undefined
Copy link

Choose a reason for hiding this comment

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

Bug: Inconsistent Session Key Causes Authentication Issues

The middleware functions in app/middleware.server.ts use a hardcoded 'sessionId' string for session retrieval. This is inconsistent with the sessionKey constant used throughout the rest of the authentication system, causing authentication failures and incorrect route protection.

Fix in Cursor Fix in Web

export const serverBuildContext = createContext<Promise<{
error: unknown
build: ServerBuild
}> | null>(null)
Copy link

Choose a reason for hiding this comment

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

Bug: Sitemap Route Fails to Access Server Build Context

The sitemap[.]xml.ts route defines its own serverBuildContext instead of importing the one created in server/index.ts. React Router contexts rely on object identity, so this separate context instance prevents the sitemap from accessing the serverBuild data set by the server.

Fix in Cursor Fix in Web


export async function loader({ request, context }: Route.LoaderArgs) {
const serverBuild = (await context.serverBuild) as { build: ServerBuild }
const serverBuild = (await (context as Readonly<RouterContextProvider>).get(
Copy link

Choose a reason for hiding this comment

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

Does this actually work?

I think the routes can accessed like this if I'm not mistaken:

const { routes } = await import('virtual:react-router/server-build')

The loadContext can be ditched completely.

)) as { build: ServerBuild }

// TODO: This is typeerror is coming up since of the remix-run/server-runtime package. We might need to remove/update that one.
// @ts-expect-error
Copy link

Choose a reason for hiding this comment

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

@kentcdodds
Copy link
Member Author

@cursor, could you review the comments and try to get things working. Don't worry about the modified seo suggestion. We can look into that in another PR.

Copy link

cursor bot commented Sep 17, 2025

Background Agent has finished! You can now access it through Cursor.
Open in Cursor


Open in Web

Learn more about Cursor Agents

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