test(solid-start): basic-auth e2e suite and example #5558
Conversation
WalkthroughAdds two complete Solid Start basic authentication example projects with server-side login/signup, password hashing, session management, protected routes via Prisma SQLite database, and end-to-end tests. Projects demonstrate authentication workflows, protected route guards, and form-based UI interactions. Changes
Sequence DiagramsequenceDiagram
actor User
participant Browser
participant Router as Solid Router
participant Root as Root Route
participant AuthGuard as _authed Route
participant LoginPage as Login Route
participant Server as Server Functions
User->>Browser: Visit /posts (protected)
Browser->>Router: Navigation request
Router->>Root: beforeLoad (fetchUser)
Root->>Server: fetchUser()
Server-->>Root: user = null
Root-->>Router: context.user = null
Router->>AuthGuard: beforeLoad check
AuthGuard-->>AuthGuard: throw 'Not authenticated'
Router->>LoginPage: errorComponent renders
LoginPage-->>Browser: Show login form
User->>Browser: Enter email/password, submit
Browser->>LoginPage: onSubmit
LoginPage->>Server: loginMutation(email, password)
Server->>Server: Find user, verify password
Server->>Server: Create/update session
Server-->>LoginPage: success
LoginPage->>Router: router.invalidate()
Router->>Root: beforeLoad (fetchUser)
Root->>Server: fetchUser()
Server-->>Root: user = {email}
Root-->>Router: context.user set
Router->>AuthGuard: beforeLoad check passes
AuthGuard->>AuthGuard: component renders
AuthGuard-->>Browser: Show /posts content
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes The PR introduces substantial new code across two mirrored project directories with intricate authentication logic, server functions, protected route guards, session management, and database integration. While most files are duplicated between e2e and examples (reducing unique reasoning), the authentication flows are non-trivial, involving password hashing, Prisma queries, session handling, and conditional UI rendering. Route tree generation, server-side data fetching, and error boundary logic require careful review for correctness and security. Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
View your CI Pipeline Execution ↗ for commit 757b46c
☁️ Nx Cloud last updated this comment at |
More templates
@tanstack/arktype-adapter
@tanstack/directive-functions-plugin
@tanstack/eslint-plugin-router
@tanstack/history
@tanstack/nitro-v2-vite-plugin
@tanstack/react-router
@tanstack/react-router-devtools
@tanstack/react-router-ssr-query
@tanstack/react-start
@tanstack/react-start-client
@tanstack/react-start-server
@tanstack/router-cli
@tanstack/router-core
@tanstack/router-devtools
@tanstack/router-devtools-core
@tanstack/router-generator
@tanstack/router-plugin
@tanstack/router-ssr-query-core
@tanstack/router-utils
@tanstack/router-vite-plugin
@tanstack/server-functions-plugin
@tanstack/solid-router
@tanstack/solid-router-devtools
@tanstack/solid-router-ssr-query
@tanstack/solid-start
@tanstack/solid-start-client
@tanstack/solid-start-server
@tanstack/start-client-core
@tanstack/start-plugin-core
@tanstack/start-server-core
@tanstack/start-static-server-functions
@tanstack/start-storage-context
@tanstack/valibot-adapter
@tanstack/virtual-file-routes
@tanstack/zod-adapter
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 35
🧹 Nitpick comments (21)
e2e/solid-start/basic-auth/public/site.webmanifest (1)
2-3: Consider adding descriptive names for better test realism.The empty
nameandshort_namefields could trigger validation warnings and don't provide a realistic test scenario. Consider using descriptive values like"Basic Auth Test"and"Auth Test"to make the e2e test more representative of a real application.Apply this diff to add meaningful names:
- "name": "", - "short_name": "", + "name": "Basic Auth Test", + "short_name": "Auth Test",e2e/solid-start/basic-auth/tests/mock-db-teardown.test.ts (2)
5-5: Consider reusing the shared PrismaClient instance.A shared
prismaClientis already exported fromsrc/utils/prisma.ts. Creating a separate instance here may lead to connection pool issues or inconsistencies.Apply this diff to use the shared client:
-import { PrismaClient } from '@prisma/client' +import { prismaClient } from '../src/utils/prisma.js' - -const prismaClient = new PrismaClient()
7-21: Add explicit disconnect to prevent connection leaks.The Prisma client should be explicitly disconnected after teardown to properly close database connections and avoid potential connection pool exhaustion in the test environment.
Apply this diff to add disconnect:
teardown('cleanup test user', async () => { if ( await prismaClient.user.findUnique({ where: { email: 'test2@gmail.com', }, }) ) { await prismaClient.user.delete({ where: { email: 'test2@gmail.com', }, }) } + await prismaClient.$disconnect() })e2e/solid-start/basic-auth/src/routes/_authed/posts.$postId.tsx (1)
7-9: Remove redundant error component wrapper.
PostErrorComponentonly passes through toErrorComponentwithout adding any custom logic or UI. You can either useErrorComponentdirectly or omit theerrorComponentoption to use the default error handling.Apply this diff to use ErrorComponent directly:
-export function PostErrorComponent({ error }: ErrorComponentProps) { - return <ErrorComponent error={error} /> -} - export const Route = createFileRoute('/_authed/posts/$postId')({ loader: ({ params: { postId } }) => fetchPost({ data: postId }), - errorComponent: PostErrorComponent, + errorComponent: ErrorComponent, component: PostComponent, notFoundComponent: () => { return <NotFound>Post not found</NotFound> }, })Alternatively, remove the
errorComponentoption entirely if the default behavior is sufficient:-export function PostErrorComponent({ error }: ErrorComponentProps) { - return <ErrorComponent error={error} /> -} - export const Route = createFileRoute('/_authed/posts/$postId')({ loader: ({ params: { postId } }) => fetchPost({ data: postId }), - errorComponent: PostErrorComponent, component: PostComponent, notFoundComponent: () => { return <NotFound>Post not found</NotFound> }, })e2e/solid-start/basic-auth/.env (1)
7-7: Optional: Address linter warnings.The dotenv-linter flags two style issues:
- Missing blank line at end of file
- Quotes around the value
The quotes are actually fine and often necessary for values with special characters. However, you may want to add a trailing newline for consistency.
Apply this diff if you want to address the linter warnings:
-DATABASE_URL="file:./dev.db" +DATABASE_URL="file:./dev.db" +Note: The quotes can be removed if preferred (
DATABASE_URL=file:./dev.db), but keeping them is also valid and more defensive.e2e/solid-start/basic-auth/src/routes/login.tsx (1)
8-10: Optional: Inline the component.
LoginCompis a thin wrapper that only returns<Login />. You can simplify by passingLogindirectly to the route config, though the current pattern may be intentional for consistency with other route files.If you prefer to simplify:
export const Route = createFileRoute('/login')({ - component: LoginComp, + component: Login, }) - -function LoginComp() { - return <Login /> -}e2e/solid-start/basic-auth/src/routes/logout.tsx (1)
1-1: Address import order per ESLint rule.ESLint flagged that imports should be sorted alphabetically.
Apply this diff to fix the import order:
-import { redirect, createFileRoute } from '@tanstack/solid-router' +import { createFileRoute, redirect } from '@tanstack/solid-router'e2e/solid-start/basic-auth/src/components/NotFound.tsx (1)
3-3: Consider addingdata-testidfor e2e tests.The similar NotFound component in
e2e/solid-start/server-routes/src/components/NotFound.tsxincludesdata-testid="default-not-found-component"on the root div, which enables more reliable e2e test assertions.Apply this diff to add the test identifier:
-export function NotFound({ children }: { children?: any }) { - return ( - <div class="space-y-2 p-2"> +export function NotFound({ children }: { children?: any }) { + return ( + <div class="space-y-2 p-2" data-testid="default-not-found-component">e2e/solid-start/basic-auth/prisma/schema.prisma (1)
14-15: Drop redundant@uniqueon primary key (and consider surrogate ID).
@id;@uniqueis unnecessary and produces redundant index. Optionally, consider a surrogateidto avoid email-as-PK friction later.model User { - email String @id @unique + email String @id password String }e2e/solid-start/basic-auth/src/components/DefaultCatchBoundary.tsx (1)
17-17: Clarify error logging (optional).Add context to the log line; helps triage noisy CI logs.
- console.error(error) + console.error('DefaultCatchBoundary Error:', error)e2e/solid-start/basic-auth/src/routes/_authed/posts.tsx (1)
3-3: Prefer extensionless TS import for path-mapped modules.Avoid
.jsin TS source to keep TS/Vite path mapping clean.-import { fetchPosts } from '~/utils/posts.js' +import { fetchPosts } from '~/utils/posts'e2e/solid-start/basic-auth/tests/mock-db-setup.test.ts (1)
5-21: Disconnect Prisma after setup to avoid handle leaks.Ensure the client is closed once setup completes.
setup('create new database', async () => { @@ } }) + +// Close Prisma after setup completes +setup.afterAll(async () => { + await prismaClient.$disconnect() +})e2e/solid-start/basic-auth/src/components/Auth.tsx (1)
29-35: Optional UX: enable browser autofill.Add
autocompletehints for better e2e realism and accessibility.<input type="email" name="email" id="email" + autocomplete="email" class="px-2 py-1 w-full rounded border border-gray-500/20 bg-white dark:bg-gray-800" /> @@ <input type="password" name="password" id="password" + autocomplete="current-password" class="px-2 py-1 w-full rounded border border-gray-500/20 bg-white dark:bg-gray-800" />Also applies to: 40-46
e2e/solid-start/basic-auth/src/routes/signup.tsx (1)
23-25: Nit: fix comment to reflect hashing (and note demo-only salt).It’s PBKDF2 hashing with a static salt, not encryption. Fine for e2e, but not for prod.
- // Encrypt the password using Sha256 into plaintext + // Hash the password (PBKDF2/SHA-256 with a static demo-only salt)e2e/solid-start/basic-auth/tests/app.spec.ts (1)
34-37: Stronger assertion for redirect-to-login.Assert against the heading by role and case-insensitively for resilience.
Apply this diff:
test('Posts redirects to login when not authenticated', async ({ page }) => { await page.goto('/posts') - await expect(page.locator('h1')).toContainText('Login') + await expect(page.getByRole('heading', { name: /login/i })).toBeVisible() })e2e/solid-start/basic-auth/src/routes/__root.tsx (2)
12-14: Import order nit (ESLint import/order).Place
solid-js/webimport before the type-onlysolid-jsimport.Apply this diff:
-import type * as Solid from 'solid-js' -import { HydrationScript } from 'solid-js/web' +import { HydrationScript } from 'solid-js/web' +import type * as Solid from 'solid-js'
41-78: Manifest link has an invalid color attribute and a typo.
coloris not valid on the manifest link, and#fffffis not a valid hex color (missing one “f”). Use a meta theme-color (or correct attribute on a mask-icon if applicable).Apply this diff:
head: () => ({ meta: [ { charset: 'utf-8', }, { name: 'viewport', content: 'width=device-width, initial-scale=1', }, + { name: 'theme-color', content: '#ffffff' }, ...seo({ title: - 'TanStack Start | Type-Safe, Client-First, Full-Stack React Framework', + 'TanStack Start | Type-Safe, Client-First, Full-Stack Solid Framework', description: `TanStack Start is a type-safe, client-first, full-stack React framework. `, }), ], links: [ { rel: 'stylesheet', href: appCss }, { rel: 'apple-touch-icon', sizes: '180x180', href: '/apple-touch-icon.png', }, { rel: 'icon', type: 'image/png', sizes: '32x32', href: '/favicon-32x32.png', }, { rel: 'icon', type: 'image/png', sizes: '16x16', href: '/favicon-16x16.png', }, - { rel: 'manifest', href: '/site.webmanifest', color: '#fffff' }, + { rel: 'manifest', href: '/site.webmanifest' }, { rel: 'icon', href: '/favicon.ico' }, ], }),e2e/solid-start/basic-auth/src/routes/_authed.tsx (2)
8-12: Tighten server input validation and return a clear success payload.Validate email/password on the server and return an explicit success object to simplify client logic.
Apply this diff:
+import { z } from 'zod' @@ export const loginFn = createServerFn({ method: 'POST', }) - .inputValidator((payload: { email: string; password: string }) => payload) + .inputValidator( + z.object({ + email: z.string().email(), + password: z.string().min(6), + }).parse, + ) .handler(async ({ data }) => { @@ // Store the user's email in the session await session.update({ userEmail: user.email, }) + return { ok: true } })Also applies to: 39-46
48-61: Auth guard relies on a magic string.Comparing
error.message === 'Not authenticated'is brittle. Prefer a typed error (e.g.,class NotAuthenticatedError) or a static symbol you caninstanceof.Would you like a small patch introducing a custom error type?
e2e/solid-start/basic-auth/playwright.config.ts (1)
10-19: Add basic CI hardening (retries, trace, and server timeout).Improves flake resistance without changing local DX.
Apply this diff:
export default defineConfig({ testDir: './tests', workers: 1, - reporter: [['line']], + reporter: [['line']], use: { /* Base URL to use in actions like `await page.goto('/')`. */ baseURL, + trace: process.env.CI ? 'on-first-retry' : 'off', }, webServer: { command: `VITE_SERVER_PORT=${PORT} pnpm build && PORT=${PORT} VITE_SERVER_PORT=${PORT} pnpm start`, url: baseURL, reuseExistingServer: !process.env.CI, stdout: 'pipe', + timeout: 120_000, }, projects: [ @@ ], + retries: process.env.CI ? 2 : 0, + forbidOnly: !!process.env.CI, })Also applies to: 21-26, 28-33
e2e/solid-start/basic-auth/src/hooks/useMutation.ts (1)
7-13: Type-safety and ergonomics: drop any-casts, clear error on start, optional onError.Avoid
as any, reset previous error before mutate, and expose anonErrorhook.Apply this diff:
export function useMutation<TVariables, TData, TError = Error>(opts: { fn: (variables: TVariables) => Promise<TData> - onSuccess?: (ctx: { data: TData }) => void | Promise<void> + onSuccess?: (ctx: { data: TData }) => void | Promise<void> + onError?: (ctx: { error: TError }) => void | Promise<void> }) { const [submittedAt, setSubmittedAt] = Solid.createSignal<number | undefined>() const [variables, setVariables] = Solid.createSignal<TVariables | undefined>() const [error, setError] = Solid.createSignal<TError | undefined>() const [data, setData] = Solid.createSignal<TData | undefined>() const [status, setStatus] = Solid.createSignal< 'idle' | 'pending' | 'success' | 'error' >('idle') const mutate = async (variables: TVariables): Promise<TData | undefined> => { setStatus('pending') setSubmittedAt(Date.now()) - setVariables(variables as any) + setVariables(variables) + setError(undefined) // try { const data = await opts.fn(variables) await opts.onSuccess?.({ data }) setStatus('success') - setError(undefined) - setData(data as any) + setData(data) return data } catch (err: any) { setStatus('error') - setError(err) + setError(err) + await opts.onError?.({ error: err }) } }Also applies to: 15-31
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (9)
e2e/solid-start/basic-auth/prisma/dev.dbis excluded by!**/*.dbe2e/solid-start/basic-auth/public/android-chrome-192x192.pngis excluded by!**/*.pnge2e/solid-start/basic-auth/public/android-chrome-512x512.pngis excluded by!**/*.pnge2e/solid-start/basic-auth/public/apple-touch-icon.pngis excluded by!**/*.pnge2e/solid-start/basic-auth/public/favicon-16x16.pngis excluded by!**/*.pnge2e/solid-start/basic-auth/public/favicon-32x32.pngis excluded by!**/*.pnge2e/solid-start/basic-auth/public/favicon.icois excluded by!**/*.icoe2e/solid-start/basic-auth/public/favicon.pngis excluded by!**/*.pngpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (44)
e2e/solid-start/basic-auth/.env(1 hunks)e2e/solid-start/basic-auth/.gitignore(1 hunks)e2e/solid-start/basic-auth/.prettierignore(1 hunks)e2e/solid-start/basic-auth/package.json(1 hunks)e2e/solid-start/basic-auth/playwright.config.ts(1 hunks)e2e/solid-start/basic-auth/postcss.config.mjs(1 hunks)e2e/solid-start/basic-auth/prisma/migrations/20240811183753_init/migration.sql(1 hunks)e2e/solid-start/basic-auth/prisma/migrations/migration_lock.toml(1 hunks)e2e/solid-start/basic-auth/prisma/schema.prisma(1 hunks)e2e/solid-start/basic-auth/public/site.webmanifest(1 hunks)e2e/solid-start/basic-auth/src/components/Auth.tsx(1 hunks)e2e/solid-start/basic-auth/src/components/DefaultCatchBoundary.tsx(1 hunks)e2e/solid-start/basic-auth/src/components/Login.tsx(1 hunks)e2e/solid-start/basic-auth/src/components/NotFound.tsx(1 hunks)e2e/solid-start/basic-auth/src/hooks/useMutation.ts(1 hunks)e2e/solid-start/basic-auth/src/routeTree.gen.ts(1 hunks)e2e/solid-start/basic-auth/src/router.tsx(1 hunks)e2e/solid-start/basic-auth/src/routes/__root.tsx(1 hunks)e2e/solid-start/basic-auth/src/routes/_authed.tsx(1 hunks)e2e/solid-start/basic-auth/src/routes/_authed/posts.$postId.tsx(1 hunks)e2e/solid-start/basic-auth/src/routes/_authed/posts.index.tsx(1 hunks)e2e/solid-start/basic-auth/src/routes/_authed/posts.tsx(1 hunks)e2e/solid-start/basic-auth/src/routes/index.tsx(1 hunks)e2e/solid-start/basic-auth/src/routes/login.tsx(1 hunks)e2e/solid-start/basic-auth/src/routes/logout.tsx(1 hunks)e2e/solid-start/basic-auth/src/routes/signup.tsx(1 hunks)e2e/solid-start/basic-auth/src/styles/app.css(1 hunks)e2e/solid-start/basic-auth/src/utils/posts.ts(1 hunks)e2e/solid-start/basic-auth/src/utils/prisma.ts(1 hunks)e2e/solid-start/basic-auth/src/utils/seo.ts(1 hunks)e2e/solid-start/basic-auth/src/utils/session.ts(1 hunks)e2e/solid-start/basic-auth/tailwind.config.mjs(1 hunks)e2e/solid-start/basic-auth/tests/app.spec.ts(1 hunks)e2e/solid-start/basic-auth/tests/mock-db-setup.test.ts(1 hunks)e2e/solid-start/basic-auth/tests/mock-db-teardown.test.ts(1 hunks)e2e/solid-start/basic-auth/tsconfig.json(1 hunks)e2e/solid-start/basic-auth/vite.config.ts(1 hunks)e2e/solid-start/serialization-adapters/test-results/.last-run.json(1 hunks)e2e/solid-start/serialization-adapters/test-results/app-SSR-serialization-adapters-data-only-chromium/error-context.md(1 hunks)e2e/solid-start/serialization-adapters/test-results/app-SSR-serialization-adapters-nested-chromium/error-context.md(1 hunks)e2e/solid-start/serialization-adapters/test-results/app-SSR-serialization-adapters-stream-chromium/error-context-1.md(1 hunks)e2e/solid-start/serialization-adapters/test-results/app-SSR-serialization-adapters-stream-chromium/error-context-2.md(1 hunks)e2e/solid-start/serialization-adapters/test-results/app-server-functions-serialization-adapters-custom-error-chromium/error-context.md(1 hunks)e2e/solid-start/serialization-adapters/test-results/app-server-functions-serialization-adapters-nested-chromium/error-context.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
e2e/**
📄 CodeRabbit inference engine (AGENTS.md)
Store end-to-end tests under the e2e/ directory
Files:
e2e/solid-start/serialization-adapters/test-results/app-SSR-serialization-adapters-stream-chromium/error-context-1.mde2e/solid-start/basic-auth/prisma/schema.prismae2e/solid-start/basic-auth/src/routes/_authed/posts.$postId.tsxe2e/solid-start/basic-auth/public/site.webmanifeste2e/solid-start/basic-auth/tailwind.config.mjse2e/solid-start/basic-auth/src/utils/prisma.tse2e/solid-start/basic-auth/prisma/migrations/migration_lock.tomle2e/solid-start/basic-auth/src/components/Auth.tsxe2e/solid-start/basic-auth/src/routes/__root.tsxe2e/solid-start/basic-auth/src/routes/_authed/posts.tsxe2e/solid-start/serialization-adapters/test-results/app-server-functions-serialization-adapters-custom-error-chromium/error-context.mde2e/solid-start/basic-auth/tsconfig.jsone2e/solid-start/basic-auth/src/routes/login.tsxe2e/solid-start/basic-auth/prisma/migrations/20240811183753_init/migration.sqle2e/solid-start/basic-auth/src/router.tsxe2e/solid-start/basic-auth/src/routes/signup.tsxe2e/solid-start/basic-auth/postcss.config.mjse2e/solid-start/basic-auth/src/utils/seo.tse2e/solid-start/basic-auth/src/components/NotFound.tsxe2e/solid-start/basic-auth/tests/mock-db-teardown.test.tse2e/solid-start/basic-auth/src/routes/_authed/posts.index.tsxe2e/solid-start/basic-auth/src/hooks/useMutation.tse2e/solid-start/basic-auth/vite.config.tse2e/solid-start/basic-auth/src/utils/posts.tse2e/solid-start/basic-auth/src/components/Login.tsxe2e/solid-start/basic-auth/playwright.config.tse2e/solid-start/basic-auth/tests/app.spec.tse2e/solid-start/basic-auth/tests/mock-db-setup.test.tse2e/solid-start/basic-auth/package.jsone2e/solid-start/serialization-adapters/test-results/app-SSR-serialization-adapters-data-only-chromium/error-context.mde2e/solid-start/basic-auth/src/components/DefaultCatchBoundary.tsxe2e/solid-start/serialization-adapters/test-results/app-server-functions-serialization-adapters-nested-chromium/error-context.mde2e/solid-start/serialization-adapters/test-results/app-SSR-serialization-adapters-nested-chromium/error-context.mde2e/solid-start/basic-auth/src/routes/logout.tsxe2e/solid-start/basic-auth/src/routes/index.tsxe2e/solid-start/basic-auth/src/utils/session.tse2e/solid-start/serialization-adapters/test-results/app-SSR-serialization-adapters-stream-chromium/error-context-2.mde2e/solid-start/basic-auth/src/styles/app.csse2e/solid-start/basic-auth/src/routes/_authed.tsxe2e/solid-start/basic-auth/src/routeTree.gen.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
e2e/solid-start/basic-auth/src/routes/_authed/posts.$postId.tsxe2e/solid-start/basic-auth/src/utils/prisma.tse2e/solid-start/basic-auth/src/components/Auth.tsxe2e/solid-start/basic-auth/src/routes/__root.tsxe2e/solid-start/basic-auth/src/routes/_authed/posts.tsxe2e/solid-start/basic-auth/src/routes/login.tsxe2e/solid-start/basic-auth/src/router.tsxe2e/solid-start/basic-auth/src/routes/signup.tsxe2e/solid-start/basic-auth/src/utils/seo.tse2e/solid-start/basic-auth/src/components/NotFound.tsxe2e/solid-start/basic-auth/tests/mock-db-teardown.test.tse2e/solid-start/basic-auth/src/routes/_authed/posts.index.tsxe2e/solid-start/basic-auth/src/hooks/useMutation.tse2e/solid-start/basic-auth/vite.config.tse2e/solid-start/basic-auth/src/utils/posts.tse2e/solid-start/basic-auth/src/components/Login.tsxe2e/solid-start/basic-auth/playwright.config.tse2e/solid-start/basic-auth/tests/app.spec.tse2e/solid-start/basic-auth/tests/mock-db-setup.test.tse2e/solid-start/basic-auth/src/components/DefaultCatchBoundary.tsxe2e/solid-start/basic-auth/src/routes/logout.tsxe2e/solid-start/basic-auth/src/routes/index.tsxe2e/solid-start/basic-auth/src/utils/session.tse2e/solid-start/basic-auth/src/routes/_authed.tsxe2e/solid-start/basic-auth/src/routeTree.gen.ts
**/src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Place file-based routes under src/routes/ directories
Files:
e2e/solid-start/basic-auth/src/routes/_authed/posts.$postId.tsxe2e/solid-start/basic-auth/src/routes/__root.tsxe2e/solid-start/basic-auth/src/routes/_authed/posts.tsxe2e/solid-start/basic-auth/src/routes/login.tsxe2e/solid-start/basic-auth/src/routes/signup.tsxe2e/solid-start/basic-auth/src/routes/_authed/posts.index.tsxe2e/solid-start/basic-auth/src/routes/logout.tsxe2e/solid-start/basic-auth/src/routes/index.tsxe2e/solid-start/basic-auth/src/routes/_authed.tsx
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Use workspace:* protocol for internal dependencies in package.json files
Files:
e2e/solid-start/basic-auth/package.json
🧠 Learnings (2)
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
PR: TanStack/router#0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to **/*.{ts,tsx} : Use TypeScript in strict mode with extensive type safety across the codebase
Applied to files:
e2e/solid-start/basic-auth/tsconfig.json
📚 Learning: 2025-10-01T18:31:35.420Z
Learnt from: schiller-manuel
PR: TanStack/router#5330
File: e2e/react-start/custom-basepath/src/routeTree.gen.ts:58-61
Timestamp: 2025-10-01T18:31:35.420Z
Learning: Do not review files named `routeTree.gen.ts` in TanStack Router repositories, as these are autogenerated files that should not be manually modified.
Applied to files:
e2e/solid-start/basic-auth/src/routeTree.gen.ts
🧬 Code graph analysis (18)
e2e/solid-start/basic-auth/src/routes/_authed/posts.$postId.tsx (3)
e2e/solid-start/basic-auth/src/routes/_authed.tsx (1)
Route(48-61)e2e/solid-start/basic-auth/src/utils/posts.ts (1)
fetchPost(11-27)e2e/solid-start/basic-auth/src/components/NotFound.tsx (1)
NotFound(3-25)
e2e/solid-start/basic-auth/src/routes/__root.tsx (5)
e2e/solid-start/basic-auth/src/utils/session.ts (1)
useAppSession(9-13)e2e/solid-start/basic-auth/src/routes/_authed.tsx (1)
Route(48-61)e2e/solid-start/basic-auth/src/utils/seo.ts (1)
seo(1-33)e2e/solid-start/basic-auth/src/components/DefaultCatchBoundary.tsx (1)
DefaultCatchBoundary(10-53)e2e/solid-start/basic-auth/src/components/NotFound.tsx (1)
NotFound(3-25)
e2e/solid-start/basic-auth/src/routes/_authed/posts.tsx (4)
e2e/solid-start/basic-auth/src/routes/_authed/posts.index.tsx (1)
Route(7-9)e2e/solid-start/basic-auth/src/routes/_authed/posts.$postId.tsx (1)
Route(11-18)e2e/solid-start/basic-auth/src/routes/_authed.tsx (1)
Route(48-61)e2e/solid-start/basic-auth/src/utils/posts.ts (1)
fetchPosts(29-37)
e2e/solid-start/basic-auth/src/routes/login.tsx (2)
e2e/solid-start/basic-auth/src/routes/_authed.tsx (1)
Route(48-61)e2e/solid-start/basic-auth/src/components/Login.tsx (1)
Login(8-71)
e2e/solid-start/basic-auth/src/router.tsx (2)
e2e/solid-start/basic-auth/src/components/DefaultCatchBoundary.tsx (1)
DefaultCatchBoundary(10-53)e2e/solid-start/basic-auth/src/components/NotFound.tsx (1)
NotFound(3-25)
e2e/solid-start/basic-auth/src/routes/signup.tsx (4)
e2e/solid-start/basic-auth/src/utils/prisma.ts (2)
prismaClient(4-4)hashPassword(6-16)e2e/solid-start/basic-auth/src/utils/session.ts (1)
useAppSession(9-13)e2e/solid-start/basic-auth/src/hooks/useMutation.ts (1)
useMutation(3-41)e2e/solid-start/basic-auth/src/components/Auth.tsx (1)
Auth(3-59)
e2e/solid-start/basic-auth/src/components/NotFound.tsx (2)
e2e/solid-start/server-routes/src/components/NotFound.tsx (1)
NotFound(3-25)examples/solid/start-basic/src/components/NotFound.tsx (1)
NotFound(3-25)
e2e/solid-start/basic-auth/tests/mock-db-teardown.test.ts (1)
e2e/solid-start/basic-auth/src/utils/prisma.ts (1)
prismaClient(4-4)
e2e/solid-start/basic-auth/src/routes/_authed/posts.index.tsx (3)
e2e/solid-start/basic-auth/src/routes/_authed/posts.tsx (1)
Route(36-39)e2e/solid-start/basic-auth/src/routes/_authed/posts.$postId.tsx (1)
Route(11-18)e2e/solid-start/basic-auth/src/routes/_authed.tsx (1)
Route(48-61)
e2e/solid-start/basic-auth/src/components/Login.tsx (4)
e2e/solid-start/basic-auth/src/hooks/useMutation.ts (1)
useMutation(3-41)e2e/solid-start/basic-auth/src/routes/_authed.tsx (1)
loginFn(8-46)e2e/solid-start/basic-auth/src/routes/signup.tsx (1)
signupFn(9-66)e2e/solid-start/basic-auth/src/components/Auth.tsx (1)
Auth(3-59)
e2e/solid-start/basic-auth/tests/app.spec.ts (1)
packages/start-plugin-core/src/schema.ts (1)
Page(200-200)
e2e/solid-start/basic-auth/tests/mock-db-setup.test.ts (1)
e2e/solid-start/basic-auth/src/utils/prisma.ts (1)
prismaClient(4-4)
e2e/solid-start/basic-auth/src/components/DefaultCatchBoundary.tsx (2)
e2e/solid-start/server-routes/src/components/DefaultCatchBoundary.tsx (1)
DefaultCatchBoundary(10-53)examples/solid/start-basic/src/components/DefaultCatchBoundary.tsx (1)
DefaultCatchBoundary(10-53)
e2e/solid-start/basic-auth/src/routes/logout.tsx (1)
e2e/solid-start/basic-auth/src/utils/session.ts (1)
useAppSession(9-13)
e2e/solid-start/basic-auth/src/routes/index.tsx (2)
e2e/solid-start/basic-auth/src/routes/__root.tsx (1)
Route(33-88)e2e/solid-start/basic-auth/src/routes/login.tsx (1)
Route(4-6)
e2e/solid-start/basic-auth/src/utils/session.ts (1)
packages/start-server-core/src/request-response.ts (1)
useSession(274-279)
e2e/solid-start/basic-auth/src/routes/_authed.tsx (3)
e2e/solid-start/basic-auth/src/utils/prisma.ts (2)
prismaClient(4-4)hashPassword(6-16)e2e/solid-start/basic-auth/src/utils/session.ts (1)
useAppSession(9-13)e2e/solid-start/basic-auth/src/components/Login.tsx (1)
Login(8-71)
e2e/solid-start/basic-auth/src/routeTree.gen.ts (2)
e2e/react-router/js-only-file-based/src/routeTree.gen.js (2)
IndexRoute(30-34)rootRouteChildren(90-94)e2e/solid-start/basic-auth/src/router.tsx (1)
getRouter(6-16)
🪛 dotenv-linter (4.0.0)
e2e/solid-start/basic-auth/.env
[warning] 7-7: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 7-7: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
🪛 ESLint
e2e/solid-start/basic-auth/src/routes/__root.tsx
[error] 13-13: solid-js/web import should occur before type import of solid-js
(import/order)
e2e/solid-start/basic-auth/src/routes/logout.tsx
[error] 1-1: Member 'createFileRoute' of the import declaration should be sorted alphabetically.
(sort-imports)
🪛 markdownlint-cli2 (0.18.1)
e2e/solid-start/serialization-adapters/test-results/app-SSR-serialization-adapters-stream-chromium/error-context-1.md
8-8: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
e2e/solid-start/serialization-adapters/test-results/app-server-functions-serialization-adapters-custom-error-chromium/error-context.md
8-8: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
e2e/solid-start/serialization-adapters/test-results/app-server-functions-serialization-adapters-nested-chromium/error-context.md
8-8: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
e2e/solid-start/serialization-adapters/test-results/app-SSR-serialization-adapters-stream-chromium/error-context-2.md
8-8: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (19)
e2e/solid-start/basic-auth/public/site.webmanifest (1)
6-11: All referenced icon files are present and correctly placed.The verification confirms that both
android-chrome-192x192.pngandandroid-chrome-512x512.pngexist in thee2e/solid-start/basic-auth/public/directory. The manifest references are valid and the web manifest will function correctly.e2e/solid-start/basic-auth/vite.config.ts (2)
1-4: LGTM!The imports are appropriate for a Vite configuration with Solid Start and TypeScript path support.
6-14: Configuration is correct per official TanStack Solid Start documentation.Both plugins are required:
tanstackStart()provides Start framework integration, andviteSolid({ ssr: true })handles Solid.js compilation with SSR—they are designed to work together, not redundantly. The plugin order in the code matches the recommended order.e2e/solid-start/basic-auth/.prettierignore (1)
1-4: LGTM — sensible Prettier ignores for generated/output files.No issues. Keeping routeTree.gen.ts out of formatting is appropriate.
e2e/solid-start/basic-auth/prisma/migrations/migration_lock.toml (1)
1-3: LGTM!Standard Prisma migration lock file correctly configured for SQLite provider. This aligns with the DATABASE_URL configuration in .env.
e2e/solid-start/basic-auth/src/routes/_authed/posts.$postId.tsx (2)
11-18: LGTM!The route configuration properly wires the loader, error handling, and not-found cases. The loader correctly extracts the
postIdparameter and the components are appropriately configured.
20-29: LGTM!The component correctly uses Solid.js reactive primitives by calling
post()as a function to access the loader data reactively.e2e/solid-start/basic-auth/postcss.config.mjs (1)
1-6: LGTM!Standard PostCSS configuration correctly set up with Tailwind CSS and Autoprefixer plugins. This aligns with the Tailwind setup in the project.
e2e/solid-start/basic-auth/src/routes/login.tsx (1)
4-6: LGTM!The login route is correctly configured and wires up the Login component appropriately.
e2e/solid-start/basic-auth/src/routes/index.tsx (2)
3-5: LGTM!Root route correctly configured for the home page path.
7-13: LGTM!Simple, appropriate home component for an e2e test application.
e2e/solid-start/basic-auth/prisma/migrations/20240811183753_init/migration.sql (1)
2-5: Passwords are properly hashed—no issues found.Verification confirms the application implements password hashing via PBKDF2 (SHA256, 100,000 iterations) in
e2e/solid-start/basic-auth/src/utils/prisma.ts. ThehashPasswordfunction usescrypto.pbkdf2to hash passwords before storage during signup and before comparison during login verification. Despite the schema storingTEXT, passwords are never persisted in plain text.e2e/solid-start/basic-auth/src/router.tsx (1)
6-16: LGTM! Clean router configuration.The router setup follows TanStack Solid Router best practices with appropriate defaults and error handling components.
e2e/solid-start/basic-auth/src/styles/app.css (1)
1-22: LGTM! Standard Tailwind configuration with dark mode support.The CSS setup includes appropriate base layer customization for color schemes and accessibility.
e2e/solid-start/basic-auth/src/routes/logout.tsx (1)
6-14: LGTM! Logout logic is correct.The server function properly clears the session and redirects to the home page.
e2e/solid-start/basic-auth/src/routes/_authed/posts.index.tsx (1)
3-9: LGTM! Simple placeholder component.The posts index route is appropriately minimal for an e2e test scaffold.
e2e/solid-start/basic-auth/tailwind.config.mjs (1)
1-4: LGTM! Standard Tailwind configuration.The content pattern appropriately scans all JS/TS/JSX/TSX files in the src directory.
e2e/solid-start/basic-auth/tsconfig.json (1)
1-23: LGTM! TypeScript configuration follows coding guidelines.The configuration correctly enables strict mode with comprehensive type safety, aligning with the project's coding standards. SolidJS JSX settings and path aliases are properly configured.
Based on coding guidelines
e2e/solid-start/basic-auth/src/routeTree.gen.ts (1)
1-225: Autogenerated route tree — skipping reviewThis file is generated by TanStack Router and shouldn’t be hand‑edited. Keeping it checked in for the e2e fixture is fine; exclude from lint/format as indicated. No action requested. Based on learnings
| !.env | ||
| .DS_Store | ||
| .cache | ||
| .vercel | ||
| .output | ||
|
|
There was a problem hiding this comment.
Do not un-ignore .env — risk of committing secrets.
Replace the negate rule with ignores and keep only an example file tracked.
-!.env
+.env
+.env.*
+!.env.example
.DS_Store
.cache
.vercel
.outputOptional: also ignore common debug logs
- pnpm-debug.log*
- npm-debug.log*
- yarn-error.log*
📝 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.
| !.env | |
| .DS_Store | |
| .cache | |
| .vercel | |
| .output | |
| .env | |
| .env.* | |
| !.env.example | |
| .DS_Store | |
| .cache | |
| .vercel | |
| .output | |
🤖 Prompt for AI Agents
In e2e/solid-start/basic-auth/.gitignore around lines 5 to 10, the .gitignore
currently un-ignores .env (using "! .env") which risks committing secrets;
replace the negate rule by re-adding ".env" to the ignore list, create and track
a safe ".env.example" with placeholder values instead, and optionally append
common debug log ignores such as "pnpm-debug.log*", "npm-debug.log*", and
"yarn-error.log*" to the .gitignore.
| "build": "vite build", | ||
| "start": "node .output/server/index.mjs", | ||
| "prisma-generate": "prisma generate", | ||
| "test:e2e": "exit 0; rm -rf port*.txt; pnpm run prisma-generate && playwright test --project=chromium" |
There was a problem hiding this comment.
Tests never run: exit 0 short‑circuits the e2e script.
Remove it so Playwright actually executes.
- "test:e2e": "exit 0; rm -rf port*.txt; pnpm run prisma-generate && playwright test --project=chromium"
+ "test:e2e": "rm -rf port*.txt && pnpm run prisma-generate && playwright test --project=chromium"📝 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.
| "test:e2e": "exit 0; rm -rf port*.txt; pnpm run prisma-generate && playwright test --project=chromium" | |
| "test:e2e": "rm -rf port*.txt && pnpm run prisma-generate && playwright test --project=chromium" |
🤖 Prompt for AI Agents
In e2e/solid-start/basic-auth/package.json around line 12, the "test:e2e" script
currently prefixes the command with "exit 0" which short-circuits the script and
prevents Playwright from running; remove the "exit 0;" prefix so the script runs
the cleanup, prisma-generate, and then executes "playwright test
--project=chromium" in sequence (keeping the rm -rf and pnpm run prisma-generate
steps intact).
| "@tanstack/solid-router": "workspace:^", | ||
| "@tanstack/solid-router-devtools": "workspace:^", | ||
| "@tanstack/solid-start": "workspace:^", | ||
| "prisma": "^5.22.0", |
There was a problem hiding this comment.
🧩 Analysis chain
Use workspace:* for internal deps (per repo guidelines).
Align all internal TanStack packages to workspace:*.
As per coding guidelines.
"dependencies": {
"@prisma/client": "5.22.0",
- "@tanstack/solid-router": "workspace:^",
- "@tanstack/solid-router-devtools": "workspace:^",
- "@tanstack/solid-start": "workspace:^",
+ "@tanstack/solid-router": "workspace:*",
+ "@tanstack/solid-router-devtools": "workspace:*",
+ "@tanstack/solid-start": "workspace:*",
@@
"devDependencies": {
@@
- "@tanstack/router-e2e-utils": "workspace:^",
+ "@tanstack/router-e2e-utils": "workspace:*",Also applies to: 27-27
I'll verify the review comment by checking the current state of the file and examining patterns across the repository.
🏁 Script executed:
#!/bin/bash
# Check the actual content of the target package.json file
echo "=== Target file content (lines 16-27) ==="
sed -n '16,27p' e2e/solid-start/basic-auth/package.json
echo -e "\n=== All package.json files in e2e/ directory ==="
find e2e -name "package.json" -type f
echo -e "\n=== Checking workspace protocol usage in all e2e package.json files ==="
for file in $(find e2e -name "package.json" -type f); do
echo "File: $file"
grep -E '@tanstack/.*workspace:\^|@tanstack/.*workspace:\*' "$file" || echo " No TanStack workspace deps found"
done
echo -e "\n=== Checking workspace protocol usage in root packages ==="
find . -maxdepth 2 -name "package.json" -type f | xargs grep -l '@tanstack' | head -5 | while read file; do
echo "File: $file"
grep -E '@tanstack/.*workspace:\^|@tanstack/.*workspace:\*' "$file" | head -3 || echo " No matches"
doneLength of output: 18194
Change all internal TanStack dependencies from workspace:^ to workspace:* per repository guidelines.
This pattern violation is systematic across the entire e2e/ directory. All internal TanStack packages must use workspace:*. Update e2e/solid-start/basic-auth/package.json along with all other e2e package.json files to align with repository standards and the root package.json pattern.
"dependencies": {
"@prisma/client": "5.22.0",
- "@tanstack/solid-router": "workspace:^",
- "@tanstack/solid-router-devtools": "workspace:^",
- "@tanstack/solid-start": "workspace:^",
+ "@tanstack/solid-router": "workspace:*",
+ "@tanstack/solid-router-devtools": "workspace:*",
+ "@tanstack/solid-start": "workspace:*",
},
"devDependencies": {
- "@tanstack/router-e2e-utils": "workspace:^",
+ "@tanstack/router-e2e-utils": "workspace:*",Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In e2e/solid-start/basic-auth/package.json around lines 16 to 19 the internal
TanStack dependencies use the incorrect version range "workspace:^"; update each
internal TanStack entry ("@tanstack/solid-router",
"@tanstack/solid-router-devtools", "@tanstack/solid-start") to use "workspace:*"
instead of "workspace:^". Also scan and update all other package.json files
under the e2e/ directory to replace any "workspace:^" references for internal
TanStack packages with "workspace:*" so they match the repository root pattern.
| -- CreateIndex | ||
| CREATE UNIQUE INDEX "User_email_key" ON "User"("email"); |
There was a problem hiding this comment.
Redundant unique index on primary key column.
The User_email_key unique index is redundant since email is already defined as the PRIMARY KEY (Line 3), which implicitly guarantees uniqueness and creates an index.
Apply this diff to remove the redundant index:
-
--- CreateIndex
-CREATE UNIQUE INDEX "User_email_key" ON "User"("email");📝 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.
| -- CreateIndex | |
| CREATE UNIQUE INDEX "User_email_key" ON "User"("email"); |
🤖 Prompt for AI Agents
In
e2e/solid-start/basic-auth/prisma/migrations/20240811183753_init/migration.sql
around lines 7-8, the CREATE UNIQUE INDEX "User_email_key" on "User"(email) is
redundant because email is already the PRIMARY KEY; remove that CREATE UNIQUE
INDEX statement from the migration file so the primary key's implicit index is
used instead.
| onSubmit: (e: HTMLFormElement) => void | ||
| status: 'pending' | 'idle' | 'success' | 'error' | ||
| afterSubmit?: JSX.Element |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Fix onSubmit typing and pass the form element (strict mode).
Prop type promises an HTMLFormElement but you pass the event; remove any and forward the form.
}: {
actionText: string
- onSubmit: (e: HTMLFormElement) => void
+ onSubmit: (form: HTMLFormElement) => void
status: 'pending' | 'idle' | 'success' | 'error'
afterSubmit?: JSX.Element
}) {
@@
- <form
- onSubmit={(e: any) => {
+ <form
+ onSubmit={(e) => {
e.preventDefault()
- onSubmit(e)
+ onSubmit(e.currentTarget as HTMLFormElement)
}}
class="space-y-4"
>Also applies to: 19-22
🤖 Prompt for AI Agents
In e2e/solid-start/basic-auth/src/components/Auth.tsx around lines 10-12 (and
similarly lines 19-22), the onSubmit prop is typed as accepting an
HTMLFormElement but the component passes the submit event (and uses any) —
update the prop signature to accept the form element (e.g., onSubmit: (form:
HTMLFormElement) => void), remove any usages of any, and change the submit
handler to call props.onSubmit(e.currentTarget as HTMLFormElement) (or extract
the form element and forward it) so the actual HTMLFormElement is passed in
strict mode.
| ``` | ||
| Error: page.goto: net::ERR_CONNECTION_REFUSED at http://localhost:49955/ssr/nested | ||
| Call log: | ||
| - navigating to "http://localhost:49955/ssr/nested", waiting until "load" | ||
|
|
||
| at /Users/admin/repos/tanstack/router/e2e/solid-start/serialization-adapters/tests/app.spec.ts:72:16 | ||
| ``` |
There was a problem hiding this comment.
Fix markdown linting error: specify language in fenced code block.
The error details block is missing a language identifier in the fence marker.
Apply this diff:
- ```
+ ```text
Error: page.goto: net::ERR_CONNECTION_REFUSED at http://localhost:49955/ssr/nested🤖 Prompt for AI Agents
In
e2e/solid-start/serialization-adapters/test-results/app-SSR-serialization-adapters-nested-chromium/error-context.md
around lines 8-14, the fenced code block containing the error details is missing
a language identifier; update the opening fence to include a language (e.g.,
```text) so the block starts with ```text and keep the rest of the block
unchanged, ensuring the fence markers match and the file passes markdown
linting.
| # Test info | ||
|
|
||
| - Name: SSR serialization adapters >> stream | ||
| - Location: /Users/admin/repos/tanstack/router/e2e/solid-start/serialization-adapters/tests/app.spec.ts:65:3 | ||
|
|
||
| # Error details | ||
|
|
||
| ``` | ||
| Error: expect(received).toEqual(expected) // deep equality | ||
|
|
||
| - Expected - 1 | ||
| + Received + 3 | ||
|
|
||
| - Array [] | ||
| + Array [ | ||
| + "Failed to load resource: net::ERR_INCOMPLETE_CHUNKED_ENCODING", | ||
| + ] | ||
| at Object.page (/Users/admin/repos/tanstack/router/e2e/e2e-utils/dist/esm/fixture.js:18:27) | ||
| ``` | ||
|
|
There was a problem hiding this comment.
Remove generated test result artifacts.
This file should not be committed; it's a generated test failure artifact.
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
8-8: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In
e2e/solid-start/serialization-adapters/test-results/app-SSR-serialization-adapters-stream-chromium/error-context-1.md
(lines 1-20), this file is a generated test result artifact that should not be
committed; remove the file from the repository and prevent future commits by
deleting it from the branch, adding its path to .gitignore or updating existing
ignore patterns as appropriate, and if already committed remove it from git
history or create a new commit that deletes the file and run git rm --cached if
you need to stop tracking without deleting locally.
| ``` | ||
| Error: expect(received).toEqual(expected) // deep equality | ||
|
|
||
| - Expected - 1 | ||
| + Received + 3 | ||
|
|
||
| - Array [] | ||
| + Array [ | ||
| + "Failed to load resource: net::ERR_INCOMPLETE_CHUNKED_ENCODING", | ||
| + ] | ||
| at Object.page (/Users/admin/repos/tanstack/router/e2e/e2e-utils/dist/esm/fixture.js:18:27) | ||
| ``` |
There was a problem hiding this comment.
Fix markdown linting error: specify language in fenced code block.
- ```
+ ```text
Error: expect(received).toEqual(expected) // deep equality🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
8-8: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In
e2e/solid-start/serialization-adapters/test-results/app-SSR-serialization-adapters-stream-chromium/error-context-1.md
around lines 8 to 19, the fenced code block is missing a language tag which
causes markdown linting errors; update the opening fence from ``` to ```text (or
another appropriate language identifier) so the block reads ```text and ensure
any other fenced blocks in this file also include a language tag.
| # Test info | ||
|
|
||
| - Name: SSR serialization adapters >> stream | ||
| - Location: /Users/admin/repos/tanstack/router/e2e/solid-start/serialization-adapters/tests/app.spec.ts:65:3 | ||
|
|
||
| # Error details | ||
|
|
||
| ``` | ||
| Error: page.goto: Test timeout of 30000ms exceeded. | ||
| Call log: | ||
| - navigating to "http://localhost:49955/ssr/stream", waiting until "load" | ||
|
|
||
| at /Users/admin/repos/tanstack/router/e2e/solid-start/serialization-adapters/tests/app.spec.ts:66:16 | ||
| ``` | ||
|
|
There was a problem hiding this comment.
Remove generated test result artifacts.
This file should not be committed; it's a generated test failure artifact.
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
8-8: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In
e2e/solid-start/serialization-adapters/test-results/app-SSR-serialization-adapters-stream-chromium/error-context-2.md
lines 1-15: this is a generated test result artifact that should not be
committed; delete this file from the branch and add an appropriate pattern to
.gitignore (e.g., the test-results directory or matching *.md under
test-results) so future test artifacts are not tracked, then commit the
.gitignore change and the deletion.
| ``` | ||
| Error: page.goto: Test timeout of 30000ms exceeded. | ||
| Call log: | ||
| - navigating to "http://localhost:49955/ssr/stream", waiting until "load" | ||
|
|
||
| at /Users/admin/repos/tanstack/router/e2e/solid-start/serialization-adapters/tests/app.spec.ts:66:16 | ||
| ``` |
There was a problem hiding this comment.
Fix markdown linting error: specify language in fenced code block.
- ```
+ ```text
Error: page.goto: Test timeout of 30000ms exceeded.🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
8-8: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In
e2e/solid-start/serialization-adapters/test-results/app-SSR-serialization-adapters-stream-chromium/error-context-2.md
around lines 8 to 14, the fenced code block is missing a language specifier
which violates markdown linting; update the opening fence from ``` to ```text
(or another appropriate language token) so the block becomes ```text and
preserves the existing content, then save the file and re-run the linter to
confirm the error is resolved.
birkskyum
changed the title
There was a problem hiding this comment.
Actionable comments posted: 36
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
examples/react/start-basic-auth/.gitignore (1)
5-5: Remove the!.envnegation rule — same critical security issue as flagged previously.The .gitignore file should prevent Git from tracking .env files, ensuring secrets don't get committed to the repository history. While this file is being modified in this PR, the unsafe
!.envnegation rule persists. This should be fixed now.Replace the negation rule with proper ignores:
-!.env +.env +.env.* +!.env.example .DS_Storeexamples/react/start-basic-auth/src/routes/_authed.tsx (1)
29-37: Critical: Two security vulnerabilities in password verification — timing attack and hardcoded salt.The password verification code has two critical security flaws:
- Timing attack vulnerability (line 32): Direct equality comparison leaks timing information attackers can exploit to guess passwords. Use constant-time comparison instead.
- Hardcoded salt in pbkdf2 (utils/prisma.ts line 8): The salt is hardcoded as
'salt'instead of per-user random. This defeats the purpose of salting—all users generate identical hashes, enabling precomputation and rainbow table attacks.Fix line 32 with constant-time comparison:
// Check if the password is correct const hashedPassword = await hashPassword(data.password) - if (user.password !== hashedPassword) { + const crypto = await import('crypto') + if (!crypto.timingSafeEqual( + Buffer.from(user.password), + Buffer.from(hashedPassword) + )) { return { error: true, message: 'Incorrect password', } }Fix utils/prisma.ts line 8 to use a random per-user salt:
- crypto.pbkdf2(password, 'salt', 100000, 64, 'sha256', (err, derivedKey) => { + const salt = crypto.randomBytes(16); + crypto.pbkdf2(password, salt, 100000, 64, 'sha256', (err, derivedKey) => {Update storage to persist the salt alongside the derived key (e.g., concatenate or use a structured format).
♻️ Duplicate comments (24)
e2e/solid-start/basic-auth/.gitignore (1)
5-5: Remove the!.envnegation rule — this risks committing secrets and contradicts the previous review feedback.The .gitignore file should prevent Git from tracking .env files, ensuring secrets don't get committed to the repository history. The negation rule on this line un-ignores the
.envfile, allowing it to be tracked and committed. This is a critical security risk that was already flagged in the previous review and must be resolved.Replace the negation rule with proper ignores and add an example file:
-!.env +.env +.env.* +!.env.example .DS_StoreAdditionally, create and track a safe
.env.examplefile at the same directory level with placeholder values instead of real secrets.examples/solid/start-basic-auth/.gitignore (1)
5-5: Remove the!.envnegation rule — this risks committing secrets and replicates the issue flagged in the previous review.The .gitignore file should prevent Git from tracking .env files, ensuring secrets don't get committed to the repository history. The negation rule on this line un-ignores the
.envfile, allowing it to be tracked and committed. This is a critical security risk.Replace the negation rule with proper ignores and add an example file:
-!.env +.env +.env.* +!.env.example .DS_StoreAdditionally, create and track a safe
.env.examplefile at the same directory level with placeholder values instead of real secrets.e2e/solid-start/basic-auth/src/utils/session.ts (2)
1-2: Rename tosession.server.tsto prevent client bundling.The file imports from
@tanstack/solid-start/server, which is server-only. Per Solid Start conventions, rename tosession.server.tsand update all import statements accordingly.
3-12: Critical: Remove hardcoded session secret and unnecessary Prisma dependency.The hardcoded password is a security risk. Additionally, importing
@prisma/clientjust for a string type is unnecessary coupling.Apply this diff:
-import type { User } from '@prisma/client' - type SessionUser = { - userEmail: User['email'] + userEmail: string } export function useAppSession() { + const password = process.env.SESSION_PASSWORD + if (!password) { + throw new Error('SESSION_PASSWORD environment variable is required') + } - return useSession<SessionUser>({ - password: 'ChangeThisBeforeShippingToProdOrYouWillBeFired', - }) + return useSession<SessionUser>({ password }) }e2e/solid-start/basic-auth/package.json (2)
12-12: Critical: Removeexit 0which prevents e2e tests from running.The test script will always exit successfully without running Playwright tests due to the
exit 0prefix.Apply this diff:
- "test:e2e": "exit 0; rm -rf port*.txt; pnpm run prisma-generate && playwright test --project=chromium" + "test:e2e": "rm -rf port*.txt && pnpm run prisma-generate && playwright test --project=chromium"
16-18: Useworkspace:*for all internal TanStack dependencies.Repository guidelines require
workspace:*instead ofworkspace:^for internal packages.As per coding guidelines.
Apply this diff:
"dependencies": { "@prisma/client": "5.22.0", - "@tanstack/solid-router": "workspace:^", - "@tanstack/solid-router-devtools": "workspace:^", - "@tanstack/solid-start": "workspace:^", + "@tanstack/solid-router": "workspace:*", + "@tanstack/solid-router-devtools": "workspace:*", + "@tanstack/solid-start": "workspace:*", "prisma": "^5.22.0","devDependencies": { "@playwright/test": "^1.50.1", - "@tanstack/router-e2e-utils": "workspace:^", + "@tanstack/router-e2e-utils": "workspace:*", "@types/node": "^22.10.2",Also applies to: 27-27
e2e/solid-start/basic-auth/src/utils/posts.ts (3)
29-37: Add timeout to prevent hanging requests.The
axios.getcall lacks a timeout, which could cause the request to hang indefinitely if the external API is unresponsive.
[raise_recommended_refactor]
5-9: Correct the type ofidto match JSONPlaceholder API.The JSONPlaceholder API returns
idas a number, not a string. This type mismatch can lead to runtime issues when consuming the API response.
11-27: Validate input and fix error status access.Two issues:
- The
inputValidatordoes not validate thatpostIdis a positive integer, allowing invalid input to reach the handler.- Line 20 accesses
err.status, but axios/redaxios expose HTTP status viaerr.response?.statuswith fallback toerr.status.These issues can lead to invalid API calls and incorrect error handling.
e2e/solid-start/basic-auth/prisma/migrations/20240811183753_init/migration.sql (1)
7-8: Remove redundant unique index on primary key.The
User_email_keyunique index is redundant becausee2e/solid-start/basic-auth/tests/mock-db-teardown.test.ts (1)
7-7: Fix misleading test description.The teardown is named
'create new database'but actually deletes a test user. The description should accurately reflect the cleanup action.e2e/solid-start/basic-auth/src/utils/prisma.ts (1)
8-8: Hardcoded salt (already flagged previously).Reiterating: using a fixed salt ('salt') is a critical vulnerability. Please address with per-user salts as already suggested in the earlier review.
e2e/solid-start/basic-auth/src/utils/seo.ts (1)
12-30: Fix Open Graph attributes and prevent undefined meta content.Open Graph tags must use
propertyinstead ofname, and optional tags should only be added when values are defined to avoidcontent="undefined".Apply this diff:
export const seo = ({ title, description, keywords, image, }: { title: string description?: string image?: string keywords?: string }) => { - const tags = [ - { title }, - { name: 'description', content: description }, - { name: 'keywords', content: keywords }, - { name: 'twitter:title', content: title }, - { name: 'twitter:description', content: description }, - { name: 'twitter:creator', content: '@tannerlinsley' }, - { name: 'twitter:site', content: '@tannerlinsley' }, - { name: 'og:type', content: 'website' }, - { name: 'og:title', content: title }, - { name: 'og:description', content: description }, - ...(image - ? [ - { name: 'twitter:image', content: image }, - { name: 'twitter:card', content: 'summary_large_image' }, - { name: 'og:image', content: image }, - ] - : []), - ] - - return tags + const tags: Array<Record<string, string>> = [{ title }] + + if (description) { + tags.push({ name: 'description', content: description }) + tags.push({ name: 'twitter:description', content: description }) + tags.push({ property: 'og:description', content: description }) + } + if (keywords) { + tags.push({ name: 'keywords', content: keywords }) + } + tags.push({ name: 'twitter:title', content: title }) + tags.push({ name: 'twitter:creator', content: '@tannerlinsley' }) + tags.push({ name: 'twitter:site', content: '@tannerlinsley' }) + tags.push({ property: 'og:type', content: 'website' }) + tags.push({ property: 'og:title', content: title }) + if (image) { + tags.push({ name: 'twitter:image', content: image }) + tags.push({ name: 'twitter:card', content: 'summary_large_image' }) + tags.push({ property: 'og:image', content: image }) + } + + return tags }e2e/solid-start/basic-auth/src/routes/_authed/posts.tsx (1)
15-21: Navigation broken: Link missing/_authedprefix.The Link points to
/posts/$postIdbut the route is defined as/_authed/posts. This will cause 404 errors when clicking post links.Apply this diff:
<Link - to="/posts/$postId" + to="/_authed/posts/$postId" params={{ postId: post.id, }}e2e/solid-start/basic-auth/src/routes/signup.tsx (1)
84-89: Client payload shape is wrong — removedata:wrapper.The mutation sends
{ data: { email, password } }butsignupFnexpects{ email, password, redirectUrl? }directly. The extra wrapper causespayload.emailto be undefined on the server.Apply this diff:
signupMutation.mutate({ - data: { - email: formData.get('email') as string, - password: formData.get('password') as string, - }, + email: formData.get('email') as string, + password: formData.get('password') as string, })e2e/solid-start/basic-auth/tests/app.spec.ts (4)
39-42: Use Playwright assertions, not truthiness on Locators.Await visibility.
- expect(page.getByText('User not found')).toBeTruthy() + await expect(page.getByText('User not found')).toBeVisible()- expect(page.getByText('Incorrect password')).toBeTruthy() + await expect(page.getByText('Incorrect password')).toBeVisible()- await login(page, 'test2@gmail.com', 'badpassword', true) - expect(page.getByText('test@gmail.com')).toBeTruthy() + await login(page, 'test2@gmail.com', 'badpassword', true) + await expect(page.getByText('test2@gmail.com')).toBeVisible()Also applies to: 46-47, 49-52
54-57: Remove hard sleep; wait on UI state.Block on a reliable post-login indicator.
- await new Promise((r) => setTimeout(r, 1000)) + await expect(page.getByRole('link', { name: 'Logout' })).toBeVisible()
4-14: Fix signup helper to use Playwright baseURL.Remove baseUrl param; navigate with relative URL.
-async function signup( - page: Page, - baseUrl: string, - email: string, - password: string, -) { - await page.goto(baseUrl + '/signup') +async function signup( + page: Page, + email: string, + password: string, +) { + await page.goto('/signup') await page.fill('input[name="email"]', email) await page.fill('input[name="password"]', password) await page.click('button[type="submit"]') }
44-47: Test logic is wrong for “incorrect password”.Create the user, then attempt a bad login.
test('Login fails with incorrect password', async ({ page }) => { - await signup(page, 'test@gmail.com', 'badpassword') - expect(page.getByText('Incorrect password')).toBeTruthy() + await signup(page, 'test@gmail.com', 'test') + await page.getByRole('link', { name: 'Logout' }).click() + await login(page, 'test@gmail.com', 'badpassword') + await expect(page.getByText('Incorrect password')).toBeVisible() })e2e/solid-start/basic-auth/src/components/Auth.tsx (1)
10-12: Pass the form element and remove any (strict mode fix).Prop type promises an HTMLFormElement but you pass the submit event; forward e.currentTarget and drop any.
Apply this diff:
export function Auth({ @@ }: { actionText: string - onSubmit: (e: HTMLFormElement) => void + onSubmit: (form: HTMLFormElement) => void status: 'pending' | 'idle' | 'success' | 'error' afterSubmit?: JSX.Element }) { @@ - <form - onSubmit={(e: any) => { - e.preventDefault() - onSubmit(e) - }} + <form + onSubmit={(e: SubmitEvent & { currentTarget: HTMLFormElement }) => { + e.preventDefault() + onSubmit(e.currentTarget) + }} class="space-y-4" >Also applies to: 19-22
e2e/solid-start/basic-auth/src/components/Login.tsx (3)
11-20: Bind server function on the client (useServerFn(loginFn)).Directly calling the server fn will fail in the client environment.
Apply this diff:
const loginMutation = useMutation({ - fn: loginFn, + fn: useServerFn(loginFn), onSuccess: async (ctx) => { if (!ctx.data?.error) { await router.invalidate() router.navigate({ to: '/' }) return } }, })
33-38: Fix login payload shape (removedata:wrapper).Matches inputValidator/handler expectations.
- loginMutation.mutate({ - data: { - email: formData.get('email') as string, - password: formData.get('password') as string, - }, - }) + loginMutation.mutate({ + email: formData.get('email') as string, + password: formData.get('password') as string, + })
53-58: Fix signup fallback payload (removedata:wrapper).Keep shape consistent with
signupFn.- signupMutation.mutate({ - data: { - email: formData.get('email') as string, - password: formData.get('password') as string, - }, - }) + signupMutation.mutate({ + email: formData.get('email') as string, + password: formData.get('password') as string, + })e2e/solid-start/basic-auth/src/routes/__root.tsx (1)
101-108: HeadContent should be in , not .Meta/link tags rendered by HeadContent won't land in the document head here (hurts SEO and correctness). Move it under .
Apply this diff:
return ( <html> <head> <HydrationScript /> + <HeadContent /> </head> <body> - <HeadContent /> <div class="p-2 flex gap-2 text-lg">
🧹 Nitpick comments (36)
examples/react/start-basic-auth/playwright.config.ts (1)
21-26: Consider verifying the necessity of duplicate environment variables.The command sets
VITE_SERVER_PORTtwice (before build and before start) and also setsPORTbefore start. While this is a safe pattern that ensures compatibility if different parts of the application use different variables, you might want to verify whether all three assignments are necessary or if the command can be simplified.For example, if only one variable is actually used at runtime:
- command: `VITE_SERVER_PORT=${PORT} pnpm build && PORT=${PORT} VITE_SERVER_PORT=${PORT} pnpm start`, + command: `VITE_SERVER_PORT=${PORT} pnpm build && VITE_SERVER_PORT=${PORT} pnpm start`,The current implementation works correctly and is safe, so this is purely an optional optimization.
examples/react/start-basic-auth/tests/mock-db-teardown.test.ts (1)
8-20: Code duplication: identical logic in setup and teardown.Both the setup and teardown files contain the exact same user deletion logic. This is redundant—typically, setup should prepare the test environment and teardown should clean up afterward. Having both do the same thing suggests either the teardown is unnecessary or the logic should be consolidated.
Consider one of these approaches:
- Remove this teardown file entirely if the setup adequately ensures a clean state before each test run
- Extract the shared deletion logic to a utility function that both files can import
- Have setup only prepare data and teardown only clean up, ensuring clear separation of concerns
examples/react/start-basic-auth/tests/mock-db-setup.test.ts (1)
8-20: Code duplication: identical logic in setup and teardown.This setup file and the teardown file (
mock-db-teardown.test.ts) contain identical user deletion logic. Consider extracting this to a shared utility function to follow DRY principles.Create a shared utility file (e.g.,
tests/utils/test-db.ts):import { PrismaClient } from '@prisma/client' export async function cleanupTestUser(email: string = 'test2@gmail.com') { const prismaClient = new PrismaClient() try { const user = await prismaClient.user.findUnique({ where: { email }, }) if (user) { await prismaClient.user.delete({ where: { email }, }) } } finally { await prismaClient.$disconnect() } }Then update both setup and teardown files to use it:
import { test as setup } from '@playwright/test' import { cleanupTestUser } from './utils/test-db' setup('ensure clean test state', async () => { await cleanupTestUser() })examples/solid/start-basic-auth/src/utils/session.ts (1)
9-13: Replace hardcoded session password with environment variableWhile the hardcoded password is acceptable for a local example, it's better to demonstrate secure practices by using an environment variable, even in example code.
Apply this diff to use an environment variable:
export function useAppSession() { return useSession<SessionUser>({ - password: 'ChangeThisBeforeShippingToProdOrYouWillBeFired', + password: process.env.SESSION_SECRET || 'ChangeThisBeforeShippingToProdOrYouWillBeFired', }) }Then add to the example's
.env.example:SESSION_SECRET=your-secret-key-here-min-32-charsThis demonstrates the proper pattern while maintaining the fallback warning for developers who forget to configure it.
examples/solid/start-basic-auth/public/site.webmanifest (1)
2-3: Provide meaningful name values for the PWA manifest.The
nameandshort_namefields are empty. For a functional PWA manifest, these should contain descriptive values like "TanStack Router Basic Auth" and "Router Auth" respectively.Apply this diff:
{ - "name": "", - "short_name": "", + "name": "TanStack Router Basic Auth Example", + "short_name": "Router Auth", "icons": [e2e/solid-start/basic-auth/src/utils/session.ts (1)
1-1: Fix the incorrect path in the file comment.The comment references
src/services/session.server.tsbut the actual file path issrc/utils/session.ts.Apply this diff:
-// src/services/session.server.ts +// src/utils/session.server.ts import { useSession } from '@tanstack/solid-start/server'examples/solid/start-basic-auth/src/routes/_authed/posts.tsx (1)
23-23: Consider adding ellipsis for truncated titles.The title is truncated at 20 characters without indicating continuation. For better UX, consider adding an ellipsis.
- <div>{post.title.substring(0, 20)}</div> + <div> + {post.title.length > 20 + ? `${post.title.substring(0, 20)}...` + : post.title} + </div>examples/solid/start-basic-auth/src/routes/_authed/posts.$postId.tsx (1)
7-9: Consider removing the unnecessary PostErrorComponent wrapper.The
PostErrorComponentonly wrapsErrorComponentwithout additional logic. You can useErrorComponentdirectly in the route configuration.-export function PostErrorComponent({ error }: ErrorComponentProps) { - return <ErrorComponent error={error} /> -} - export const Route = createFileRoute('/_authed/posts/$postId')({ loader: ({ params: { postId } }) => fetchPost({ data: postId }), - errorComponent: PostErrorComponent, + errorComponent: ErrorComponent, component: PostComponent,examples/solid/start-basic-auth/src/hooks/useMutation.ts (1)
18-18: Remove unnecessaryas anytype assertions.The type casts on lines 18 and 25 bypass TypeScript's type safety without clear necessity. The generic types should allow proper inference without these casts.
Apply this diff:
setStatus('pending') setSubmittedAt(Date.now()) - setVariables(variables as any) + setVariables(variables) // try { const data = await opts.fn(variables) await opts.onSuccess?.({ data }) setStatus('success') setError(undefined) - setData(data as any) + setData(data) return dataAlso applies to: 25-25
examples/solid/start-basic-auth/.env (1)
7-7: Consider removing quotes and adding trailing newline.Two minor style issues flagged by static analysis:
- Quotes around the value are unnecessary for this simple file path and may be included in the parsed value (depending on the parser)
- Missing blank line at end of file
While Prisma's dotenv parsing handles quotes correctly, removing them follows .env conventions more closely.
Apply this diff:
-DATABASE_URL="file:./dev.db" +DATABASE_URL=file:./dev.db +e2e/solid-start/basic-auth/tests/mock-db-teardown.test.ts (1)
8-20: Consider simplifying withdeleteMany.The current check-then-delete pattern makes two database calls. You can simplify this with a single
deleteManycall, which is idempotent and won't error if the user doesn't exist.Apply this diff:
- if ( - await prismaClient.user.findUnique({ - where: { - email: 'test2@gmail.com', - }, - }) - ) { - await prismaClient.user.delete({ - where: { - email: 'test2@gmail.com', - }, - }) - } + await prismaClient.user.deleteMany({ + where: { + email: 'test2@gmail.com', + }, + })e2e/solid-start/basic-auth/.env (1)
7-7: Add trailing newline (optional).While not functionally important, adding a blank line at the end of the file follows POSIX convention and resolves the linter warning.
The quoted value for
DATABASE_URLis acceptable—Prisma handles quotes correctly in.envfiles, so the linter warning can be safely ignored.e2e/solid-start/basic-auth/src/routes/login.tsx (1)
4-10: Consider simplifying the component reference.The
LoginCompwrapper is unnecessary. You can directly reference theLogincomponent in the route configuration.Apply this diff to simplify:
-export const Route = createFileRoute('/login')({ - component: LoginComp, -}) - -function LoginComp() { - return <Login /> -} +export const Route = createFileRoute('/login')({ + component: Login, +})examples/solid/start-basic-auth/src/routes/login.tsx (1)
4-10: Consider simplifying the component reference.The
LoginCompwrapper is unnecessary. You can directly reference theLogincomponent in the route configuration.Apply this diff to simplify:
-export const Route = createFileRoute('/login')({ - component: LoginComp, -}) - -function LoginComp() { - return <Login /> -} +export const Route = createFileRoute('/login')({ + component: Login, +})e2e/solid-start/basic-auth/src/routes/logout.tsx (1)
1-1: Fix import order per ESLint configuration.The imports should be sorted alphabetically:
createFileRouteshould come beforeredirect.Apply this diff:
-import { redirect, createFileRoute } from '@tanstack/solid-router' +import { createFileRoute, redirect } from '@tanstack/solid-router'examples/solid/start-basic-auth/src/routes/logout.tsx (2)
6-14: Consider adding error handling.If session operations fail, the error will propagate uncaught. Consider wrapping the session operations in a try-catch to handle potential failures gracefully.
Apply this diff:
const logoutFn = createServerFn({ method: 'POST' }).handler(async () => { - const session = await useAppSession() + try { + const session = await useAppSession() - session.clear() + session.clear() + } catch (error) { + // Log error but still redirect + console.error('Logout error:', error) + } throw redirect({ href: '/', }) })
16-19: Consider usingbeforeLoadinstead ofloaderfor side effects.The
loaderis typically intended for data fetching, whilebeforeLoadis more appropriate for side effects like logout operations. This makes the intent clearer and follows router conventions.Apply this diff:
export const Route = createFileRoute('/logout')({ preload: false, - loader: () => logoutFn(), + beforeLoad: () => logoutFn(), })e2e/solid-start/basic-auth/src/components/NotFound.tsx (1)
3-25: Improve type safety and add test identifier.The
childrenprop is typed asany, which bypasses TypeScript's type checking. Additionally, consider adding adata-testidattribute for easier e2e testing, as seen in the similar component ate2e/solid-start/server-routes/src/components/NotFound.tsx.Apply this diff:
+import type { JSX } from 'solid-js' + -export function NotFound({ children }: { children?: any }) { +export function NotFound({ children }: { children?: JSX.Element }) { return ( - <div class="space-y-2 p-2"> + <div class="space-y-2 p-2" data-testid="default-not-found-component"> <div class="text-gray-600 dark:text-gray-400"> {children || <p>The page you are looking for does not exist.</p>} </div>examples/solid/start-basic-auth/src/components/NotFound.tsx (1)
3-25: Improve type safety and add test identifier.The
childrenprop is typed asany, which bypasses TypeScript's type checking. Additionally, consider adding adata-testidattribute for easier e2e testing, as seen in the similar component ate2e/solid-start/server-routes/src/components/NotFound.tsx.Apply this diff:
+import type { JSX } from 'solid-js' + -export function NotFound({ children }: { children?: any }) { +export function NotFound({ children }: { children?: JSX.Element }) { return ( - <div class="space-y-2 p-2"> + <div class="space-y-2 p-2" data-testid="default-not-found-component"> <div class="text-gray-600 dark:text-gray-400"> {children || <p>The page you are looking for does not exist.</p>} </div>examples/solid/start-basic-auth/prisma/schema.prisma (1)
13-16: Consider adding audit fields and clearer password storage.Add createdAt/updatedAt and either store salt separately or store "salt:hash" to support per-user salting.
Example:
model User { - email String @id - password String + email String @id + passwordHash String + passwordSalt String + createdAt DateTime @default(now()) + updatedAt DateTime @updatedAt }e2e/solid-start/basic-auth/src/utils/prisma.ts (2)
4-4: Make PrismaClient HMR-safe to avoid multiple clients in dev.Ensure a single PrismaClient instance across reloads.
Apply:
-import { PrismaClient } from '@prisma/client' - -export const prismaClient = new PrismaClient() +import { PrismaClient } from '@prisma/client' + +declare global { + // eslint-disable-next-line no-var + var __PRISMA__: PrismaClient | undefined +} + +export const prismaClient = + globalThis.__PRISMA__ ?? new PrismaClient() + +if (process.env.NODE_ENV !== 'production') { + globalThis.__PRISMA__ = prismaClient +}
6-16: Add a dedicated password verification helper.Once per-user salts are implemented, expose verifyPassword to avoid ad-hoc comparisons at call sites.
Example:
export function hashPassword(password: string) { return new Promise<string>((resolve, reject) => { - crypto.pbkdf2(password, 'salt', 100000, 64, 'sha256', (err, derivedKey) => { + // Use a unique salt per user (pass in or generate here) + const salt = crypto.randomBytes(16).toString('hex') + crypto.pbkdf2(password, salt, 100000, 64, 'sha256', (err, derivedKey) => { if (err) { reject(err) } else { - resolve(derivedKey.toString('hex')) + resolve(`${salt}:${derivedKey.toString('hex')}`) } }) }) } + +export async function verifyPassword( + input: string, + stored: string, +) { + const [salt, hash] = stored.split(':') + const derived = await new Promise<string>((resolve, reject) => { + crypto.pbkdf2(input, salt, 100000, 64, 'sha256', (err, dk) => + err ? reject(err) : resolve(dk.toString('hex')), + ) + }) + return crypto.timingSafeEqual( + Buffer.from(hash, 'hex'), + Buffer.from(derived, 'hex'), + ) +}examples/solid/start-basic-auth/src/routes/signup.tsx (2)
23-25: Clarify (and correct) the password handling comment.This is hashing, not encryption, and never results in plaintext. Also, plan to store a per-user salt with the hash.
Apply:
- // Encrypt the password using Sha256 into plaintext + // Hash the password with a strong KDF (PBKDF2) and a per-user salt
29-47: Use a proper password check and avoid account enumeration.After implementing per-user salts, compare via a verifyPassword helper and consider generic error messaging to avoid disclosing whether an account exists.
Example:
- if (found.password !== password) { - return { - error: true, - userExists: true, - message: 'User already exists', - } - } + // Use constant-time verification + const ok = await verifyPassword(payload.password, found.password) + if (!ok) { + return { error: true, message: 'Invalid credentials' } + }examples/solid/start-basic-auth/playwright.config.ts (1)
10-26: Harden Playwright config for CI flakiness.Add retries, tracing, and webServer timeout.
Apply this diff:export default defineConfig({ testDir: './tests', workers: 1, + forbidOnly: !!process.env.CI, + retries: process.env.CI ? 2 : 0, reporter: [['line']], use: { /* Base URL to use in actions like `await page.goto('/')`. */ baseURL, + trace: 'on-first-retry', + video: process.env.CI ? 'retain-on-failure' : 'off', }, webServer: { command: `VITE_SERVER_PORT=${PORT} pnpm build && PORT=${PORT} VITE_SERVER_PORT=${PORT} pnpm start`, url: baseURL, reuseExistingServer: !process.env.CI, stdout: 'pipe', + timeout: 120_000, },Also applies to: 28-34
examples/react/start-basic-auth/tests/app.spec.ts (2)
39-42: Use Playwright assertions instead of truthiness.Await visibility to avoid false positives.
Apply these diffs:- expect(page.getByText('User not found')).toBeTruthy() + await expect(page.getByText('User not found')).toBeVisible()- expect(page.getByText('Incorrect password')).toBeTruthy() + await expect(page.getByText('Incorrect password')).toBeVisible()- expect(page.getByText('test@gmail.com')).toBeTruthy() + await expect(page.getByText('test2@gmail.com')).toBeVisible()Also applies to: 46-47, 50-52
54-57: Remove hard sleep; wait on post-login UI.Replace the timeout with a deterministic wait.
Apply this diff:test('Navigating to post after logging in', async ({ page }) => { await login(page, 'test@gmail.com', 'test') - await new Promise((r) => setTimeout(r, 1000)) + await expect(page.getByRole('link', { name: 'Logout' })).toBeVisible() await page.getByRole('link', { name: 'Posts' }).click()examples/solid/start-basic-auth/src/components/Auth.tsx (1)
29-45: Consider addingrequiredattribute to form inputs.Email and password fields lack the
requiredattribute, allowing empty form submission. Adding it improves UX with native browser validation.<input type="email" name="email" id="email" + required class="px-2 py-1 w-full rounded border border-gray-500/20 bg-white dark:bg-gray-800" /> </div> <div> <label for="password" class="block text-xs"> Password </label> <input type="password" name="password" id="password" + required class="px-2 py-1 w-full rounded border border-gray-500/20 bg-white dark:bg-gray-800" />examples/solid/start-basic-auth/src/components/DefaultCatchBoundary.tsx (1)
10-52: LGTM with a minor logging consistency suggestion.The error boundary implementation correctly handles root vs. nested route scenarios and provides good UX with "Try Again", "Home", and "Go Back" options.
For consistency with the related example at
examples/solid/start-basic/src/components/DefaultCatchBoundary.tsx, consider adding a prefix to the console.error:- console.error(error) + console.error('DefaultCatchBoundary Error:', error)examples/solid/start-basic-auth/tests/app.spec.ts (1)
54-60: Replace arbitrary delay with robust wait condition.Line 56 uses a hard-coded 1-second timeout. This is brittle and can cause flakiness or slow tests unnecessarily.
test('Navigating to post after logging in', async ({ page }) => { await login(page, 'test@gmail.com', 'test') - await new Promise((r) => setTimeout(r, 1000)) + await page.waitForSelector('text=Logout', { state: 'visible' }) await page.getByRole('link', { name: 'Posts' }).click()e2e/solid-start/basic-auth/src/routes/__root.tsx (1)
13-13: Fix import order per ESLint rules.The HydrationScript import from
solid-js/webshould come before the type import fromsolid-js.As per coding guidelines
import type * as Solid from 'solid-js' +import { HydrationScript } from 'solid-js/web' import { DefaultCatchBoundary } from '~/components/DefaultCatchBoundary.js' import { NotFound } from '~/components/NotFound.js' import appCss from '~/styles/app.css?url' import { seo } from '~/utils/seo.js' import { useAppSession } from '~/utils/session.js' -import { HydrationScript } from 'solid-js/web'e2e/solid-start/basic-auth/src/hooks/useMutation.ts (2)
15-31: Remove unnecessary type casts.Lines 18 and 25 use
as anycasts which defeat type safety. SolidJS signals should infer types correctly without casts.const mutate = async (variables: TVariables): Promise<TData | undefined> => { setStatus('pending') setSubmittedAt(Date.now()) - setVariables(variables as any) + setVariables(variables) // try { const data = await opts.fn(variables) await opts.onSuccess?.({ data }) setStatus('success') setError(undefined) - setData(data as any) + setData(data) return data
3-41: Consider adding mutation cancellation support.The hook lacks cancellation, so if a component unmounts or a new mutation starts while one is pending, the old mutation's callback can still fire and update state. This can cause race conditions.
For a production-ready hook, consider adding an AbortController-based cancellation mechanism to prevent stale mutations from updating state after unmount or new mutation initiation.
examples/solid/start-basic-auth/src/routes/__root.tsx (3)
103-109: Move HeadContent to and set html lang.Place HeadContent in the head for correct meta/link injection and add lang for a11y/SEO.
Apply:
- return ( - <html> + return ( + <html lang="en"> <head> - <HydrationScript /> + <HydrationScript /> + <HeadContent /> </head> <body> - <HeadContent /> <div class="p-2 flex gap-2 text-lg">
140-140: Gate Devtools to development builds.Avoid shipping devtools in production.
- <TanStackRouterDevtools position="bottom-right" /> + {import.meta.env.DEV && ( + <TanStackRouterDevtools position="bottom-right" /> + )}
34-41: Harden beforeLoad with graceful fallback.Network/SSR failures in fetchUser will throw and needlessly trip the error boundary. Return a null user instead.
beforeLoad: async () => { - const user = await fetchUser() - - return { - user, - } + try { + const user = await fetchUser() + return { user } + } catch (err) { + console.error('fetchUser failed', err) + return { user: null } + } },
| const formData = new FormData(e.target as unknown as HTMLFormElement) | ||
|
|
||
| loginMutation.mutate({ | ||
| data: { | ||
| email: formData.get('email') as string, | ||
| password: formData.get('password') as string, | ||
| }, | ||
| }) | ||
| }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Align onSubmit param with Auth (form element) and remove unsafe casts.
After fixing Auth, receive the form and build FormData from it.
Apply this diff:
- onSubmit={(e) => {
- const formData = new FormData(e.target as unknown as HTMLFormElement)
+ onSubmit={(form) => {
+ const formData = new FormData(form)
- loginMutation.mutate({
- data: {
- email: formData.get('email') as string,
- password: formData.get('password') as string,
- },
- })
+ loginMutation.mutate({
+ email: formData.get('email') as string,
+ password: formData.get('password') as string,
+ })
}}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In e2e/solid-start/basic-auth/src/components/Login.tsx around lines 31 to 39,
the onSubmit handler currently uses an incorrectly typed event and performs
unsafe casts when creating FormData; update the handler signature to accept the
proper form event (so the parameter is typed as an HTMLFormElement or a
SubmitEvent whose currentTarget is an HTMLFormElement), obtain the form element
from e.currentTarget (or accept the form directly) and construct new
FormData(form) without any "as unknown" or "as string" casts, then pass the
email and password values to loginMutation.mutate ensuring correct null handling
(e.g., default to empty string or validate before submit).
| // Check if the user exists | ||
| if (!user) { | ||
| return { | ||
| error: true, | ||
| userNotFound: true, | ||
| message: 'User not found', | ||
| } | ||
| } |
There was a problem hiding this comment.
User enumeration risk via distinct error responses.
The login function returns a userNotFound: true flag (line 24) that allows attackers to enumerate registered email addresses by observing different error responses. While this improves UX by suggesting signup, it's a security risk in production.
For a demo/example this may be acceptable, but for production consider returning a generic "Invalid email or password" message for both cases.
🤖 Prompt for AI Agents
In e2e/solid-start/basic-auth/src/routes/_authed.tsx around lines 20 to 27, the
handler returns a distinct userNotFound flag and message when the user is
missing, enabling user enumeration; change it to always return a generic
authentication failure response (e.g., { error: true, message: 'Invalid email or
password' }) for both missing users and bad credentials, remove the userNotFound
flag, and keep response timing consistent so the same error is returned whether
the user exists or not.
| // Check if the password is correct | ||
| const hashedPassword = await hashPassword(data.password) | ||
|
|
||
| if (user.password !== hashedPassword) { | ||
| return { | ||
| error: true, | ||
| message: 'Incorrect password', | ||
| } |
There was a problem hiding this comment.
Use timing-safe password comparison to prevent timing attacks.
Line 32 performs a simple string inequality check (!==) for password verification. This is vulnerable to timing attacks because string comparison can short-circuit, leaking information about password length and content through timing differences.
Apply this diff to use a timing-safe comparison:
- const hashedPassword = await hashPassword(data.password)
-
- if (user.password !== hashedPassword) {
+ const hashedPassword = await hashPassword(data.password)
+ const userBuffer = Buffer.from(user.password, 'hex')
+ const hashBuffer = Buffer.from(hashedPassword, 'hex')
+
+ if (userBuffer.length !== hashBuffer.length ||
+ !crypto.timingSafeEqual(userBuffer, hashBuffer)) {
return {
error: true,
message: 'Incorrect password',
}
}You'll need to import crypto at the top of the file:
+import crypto from 'node:crypto'
import { createFileRoute } from '@tanstack/solid-router'📝 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.
| // Check if the password is correct | |
| const hashedPassword = await hashPassword(data.password) | |
| if (user.password !== hashedPassword) { | |
| return { | |
| error: true, | |
| message: 'Incorrect password', | |
| } | |
| // Check if the password is correct | |
| const hashedPassword = await hashPassword(data.password) | |
| const userBuffer = Buffer.from(user.password, 'hex') | |
| const hashBuffer = Buffer.from(hashedPassword, 'hex') | |
| if (userBuffer.length !== hashBuffer.length || | |
| !crypto.timingSafeEqual(userBuffer, hashBuffer)) { | |
| return { | |
| error: true, | |
| message: 'Incorrect password', | |
| } |
🤖 Prompt for AI Agents
In e2e/solid-start/basic-auth/src/routes/_authed.tsx around lines 29 to 36,
replace the direct string inequality check for passwords with a timing-safe
comparison: import crypto at the top, convert both the stored user.password and
the computed hashedPassword to Buffers (e.g., Buffer.from(...,'utf8')), if their
lengths differ pad the shorter buffer with zeros so both buffers are equal
length, then use crypto.timingSafeEqual(bufHashedPassword, bufUserPassword) to
decide success; if timingSafeEqual returns false, return the existing error
response.
| } | ||
|
|
||
| export const Route = createFileRoute('/_authed/posts/$postId')({ | ||
| loader: ({ params: { postId } }) => fetchPost({ data: postId }), |
There was a problem hiding this comment.
Fix server function call shape in loader.
createServerFn with inputValidator((postId: string) => postId) expects a plain string argument, not an object with data.
Use:
- loader: ({ params: { postId } }) => fetchPost({ data: postId }),
+ loader: ({ params: { postId } }) => fetchPost(postId),📝 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.
| loader: ({ params: { postId } }) => fetchPost({ data: postId }), | |
| loader: ({ params: { postId } }) => fetchPost(postId), |
🤖 Prompt for AI Agents
In e2e/solid-start/basic-auth/src/routes/_authed/posts.$postId.tsx around line
12, the loader currently calls the server function with an object ({ data:
postId }) but the server function was created with inputValidator((postId:
string) => postId) and expects a plain string; change the call to pass the raw
postId string (fetchPost(postId)) so the server function receives the correct
argument shape.
| }, | ||
| }) | ||
|
|
||
| // Encrypt the password using Sha256 into plaintext |
There was a problem hiding this comment.
Misleading comment about password hashing.
The comment states "Encrypt the password using Sha256 into plaintext" but the code uses PBKDF2 key derivation (hashing), not encryption, and doesn't produce plaintext.
Clarify the comment:
- // Encrypt the password using Sha256 into plaintext
+ // Hash the password using PBKDF2 with SHA256
const password = await hashPassword(payload.password)📝 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.
| // Encrypt the password using Sha256 into plaintext | |
| // Hash the password using PBKDF2 with SHA256 |
🤖 Prompt for AI Agents
In e2e/solid-start/basic-auth/src/routes/signup.tsx around line 23, the comment
"Encrypt the password using Sha256 into plaintext" is incorrect and misleading;
replace it with a concise, accurate comment stating that the password is being
hashed/derived using PBKDF2 with SHA-256 (not encrypted) and that the result is
a derived hash (not plaintext), and mention key parameters used (salt,
iterations, key length) if present so the comment accurately reflects the
operation.
| test('Can sign up from a not found user', async ({ page }) => { | ||
| await login(page, 'test2@gmail.com', 'badpassword', true) | ||
| expect(page.getByText('test@gmail.com')).toBeTruthy() | ||
| }) |
There was a problem hiding this comment.
Fix email reference mismatch.
Line 51 checks for 'test@gmail.com' but Line 50 signs up 'test2@gmail.com'.
test('Can sign up from a not found user', async ({ page }) => {
await login(page, 'test2@gmail.com', 'badpassword', true)
- expect(page.getByText('test@gmail.com')).toBeTruthy()
+ expect(page.getByText('test2@gmail.com')).toBeTruthy()
})🤖 Prompt for AI Agents
In examples/solid/start-basic-auth/tests/app.spec.ts around lines 49 to 52, the
test signs up/logs in with 'test2@gmail.com' but the assertion checks for
'test@gmail.com'; update the assertion to check for 'test2@gmail.com' (or
alternatively change the login call to use 'test@gmail.com' so both lines
reference the same email) so the test verifies the correct user email.
|
|
||
| import { PrismaClient } from '@prisma/client' | ||
|
|
||
| const prismaClient = new PrismaClient() |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Reuse shared PrismaClient from utils.
Creating a new PrismaClient instance here can lead to connection pool exhaustion and inconsistent state. Import and use the shared prismaClient from ~/utils/prisma.ts instead.
Apply this diff:
-import { PrismaClient } from '@prisma/client'
-
-const prismaClient = new PrismaClient()
+import { prismaClient } from '~/utils/prisma'Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In examples/solid/start-basic-auth/tests/mock-db-setup.test.ts around line 5,
the file instantiates a new PrismaClient which can exhaust connection pools;
instead import and use the shared prismaClient exported from ~/utils/prisma.ts
and remove the local `new PrismaClient()` usage so all tests share the single
client instance; update the imports to include `prismaClient` and replace
references to the local variable with the shared one.
| setup('create new database', async () => { | ||
| if ( | ||
| await prismaClient.user.findUnique({ | ||
| where: { | ||
| email: 'test2@gmail.com', | ||
| }, | ||
| }) | ||
| ) { | ||
| await prismaClient.user.delete({ | ||
| where: { | ||
| email: 'test2@gmail.com', | ||
| }, | ||
| }) | ||
| } | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Misleading task name and missing cleanup.
The task is named "create new database" but only deletes a test user if it exists. Consider renaming to "clean up test user" or similar. Additionally, ensure the Prisma client is properly disconnected after the setup completes to avoid connection leaks.
Apply this diff:
-setup('create new database', async () => {
+setup('clean up test user', async () => {
if (
await prismaClient.user.findUnique({
where: {
email: 'test2@gmail.com',
},
})
) {
await prismaClient.user.delete({
where: {
email: 'test2@gmail.com',
},
})
}
+
+ await prismaClient.$disconnect()
})🤖 Prompt for AI Agents
In examples/solid/start-basic-auth/tests/mock-db-setup.test.ts around lines 7 to
21, the setup task name "create new database" is misleading and the Prisma
client is not disconnected; rename the task to something like "clean up test
user" (or similar) to reflect that it only deletes a test user if present, and
after the conditional delete ensure you call prismaClient.$disconnect() (or
await prismaClient.$disconnect()) so the Prisma connection is properly closed to
avoid leaks.
| const prismaClient = new PrismaClient() | ||
|
|
||
| teardown('create new database', async () => { | ||
| if ( | ||
| await prismaClient.user.findUnique({ | ||
| where: { | ||
| email: 'test2@gmail.com', | ||
| }, | ||
| }) | ||
| ) { | ||
| await prismaClient.user.delete({ | ||
| where: { | ||
| email: 'test2@gmail.com', | ||
| }, | ||
| }) | ||
| } | ||
| }) |
There was a problem hiding this comment.
Resource leak: PrismaClient not disconnected after teardown.
Creating a new PrismaClient instance without calling $disconnect() after teardown can leave connections open, potentially exhausting the connection pool in CI environments.
Additionally, consider importing the shared prismaClient from ~/utils/prisma instead of instantiating a new client.
Apply this diff:
-import { PrismaClient } from '@prisma/client'
-
-const prismaClient = new PrismaClient()
+import { prismaClient } from '../src/utils/prisma'
-teardown('create new database', async () => {
+teardown('cleanup test user', async () => {
- if (
- await prismaClient.user.findUnique({
- where: {
- email: 'test2@gmail.com',
- },
- })
- ) {
- await prismaClient.user.delete({
- where: {
- email: 'test2@gmail.com',
- },
- })
- }
+ await prismaClient.user.deleteMany({
+ where: {
+ email: 'test2@gmail.com',
+ },
+ })
+ await prismaClient.$disconnect()
})Note: deleteMany will no-op if the user doesn't exist, eliminating the need for the conditional check.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In examples/solid/start-basic-auth/tests/mock-db-teardown.test.ts around lines 5
to 21, the test creates a new PrismaClient instance but never disconnects it and
uses a conditional delete; replace the local new PrismaClient with the shared
prismaClient imported from ~/utils/prisma, simplify the cleanup to call
prismaClient.user.deleteMany({ where: { email: 'test2@gmail.com' } }) (no
conditional needed), and ensure the teardown calls await
prismaClient.$disconnect() (or only disconnect if this file owns the client; if
using the shared client, do not disconnect globally—adjust to call $disconnect()
only when appropriate).
|
|
||
| const prismaClient = new PrismaClient() | ||
|
|
||
| teardown('create new database', async () => { |
There was a problem hiding this comment.
Fix misleading teardown description.
The teardown block is labeled "create new database" but actually deletes a test user. This is confusing for maintainers.
Apply this diff:
-teardown('create new database', async () => {
+teardown('cleanup test user', async () => {📝 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.
| teardown('create new database', async () => { | |
| teardown('cleanup test user', async () => { |
🤖 Prompt for AI Agents
In examples/solid/start-basic-auth/tests/mock-db-teardown.test.ts around line 7,
the teardown block is incorrectly labeled "create new database" though it
deletes a test user; update the teardown description to accurately reflect its
behavior (e.g., "delete test user" or "teardown test user") so the label matches
the action and avoid confusion for maintainers.
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (7)
examples/solid/start-basic-auth/package.json (1)
14-16: Useworkspace:*protocol for internal TanStack dependencies.Internal TanStack packages should use the
workspace:*protocol instead of hardcoded versions to align with repository standards and ensure consistency across the monorepo.Apply this diff:
"dependencies": { "@prisma/client": "5.22.0", - "@tanstack/solid-router": "^1.133.20", - "@tanstack/solid-router-devtools": "^1.133.20", - "@tanstack/solid-start": "^1.133.20", + "@tanstack/solid-router": "workspace:*", + "@tanstack/solid-router-devtools": "workspace:*", + "@tanstack/solid-start": "workspace:*", "prisma": "^5.22.0",As per coding guidelines.
examples/solid/start-basic-auth/src/routes/signup.tsx (1)
81-86: Fix client call: pass the actual input, not{ data: ... }.The mutation is called with
{ data: { email, password } }, but theinputValidatoron lines 9-11 expects the payload shape{ email: string; password: string; redirectUrl?: string }directly. This mismatch will cause the server function to fail.Apply this diff:
- signupMutation.mutate({ - data: { - email: formData.get('email') as string, - password: formData.get('password') as string, - }, - }) + signupMutation.mutate({ + email: formData.get('email') as string, + password: formData.get('password') as string, + })examples/solid/start-basic-auth/src/routes/_authed.tsx (2)
7-7: Rate‑limit login attempts.Protect against brute‑force (e.g., per‑email/IP budget with sliding window).
29-34: Use constant‑time comparison for password verification.Direct string compare leaks timing information.
- if (user.password !== hashedPassword) { + const stored = Buffer.from(user.password, 'hex') + const computed = Buffer.from(hashedPassword, 'hex') + if ( + stored.length !== computed.length || + !crypto.timingSafeEqual(stored, computed) + ) { return { error: true, message: 'Incorrect password', } }Add at top:
+import crypto from 'node:crypto'examples/solid/start-basic-auth/src/routes/__root.tsx (3)
19-29: Session secret must come from env, not hardcoded helper.useAppSession reads a hardcoded password in utils/session.ts. Move to env with 32+ chars and validate before use. See prior review for example snippet.
Based on past review comments
49-53: Fix framework naming in SEO strings.
Replace “React” with “Solid” to match this example.- title: - 'TanStack Start | Type-Safe, Client-First, Full-Stack React Framework', - description: `TanStack Start is a type-safe, client-first, full-stack React framework. `, + title: + 'TanStack Start | Type-Safe, Client-First, Full-Stack Solid Framework', + description: `TanStack Start is a type-safe, client-first, full-stack Solid framework.`,
74-76: Remove invalid color on manifest link; optionally add theme-color meta.
color is not valid here and the hex is malformed.- { rel: 'manifest', href: '/site.webmanifest', color: '#fffff' }, + { rel: 'manifest', href: '/site.webmanifest' },Optionally:
head: () => ({ meta: [ { charset: 'utf-8' }, { name: 'viewport', content: 'width=device-width, initial-scale=1' }, + { name: 'theme-color', content: '#ffffff' }, ...seo({
🧹 Nitpick comments (14)
examples/solid/start-basic-auth/README.md (6)
25-25: Capitalize "Markdown" as a proper noun.Apply this diff:
-In production, the markdown doc pages are fetched from the GitHub repos of the projects, but in development they are read from the local file system. +In production, the Markdown doc pages are fetched from the GitHub repos of the projects, but in development they are read from the local file system.
45-45: Specify a language for the fenced code block.Apply this diff:
-``` +```sh mkdir tanstack -``` +```
46-52: Remove blank line inside blockquote.Blockquotes should not have internal blank lines per Markdown standards. Apply this diff:
> [!NOTE] > Your `tanstack` directory should look like this: > -> > ```
59-64: Specify a language for the fenced code block.Apply this diff:
-``` +```sh cd tanstack.com
66-66: Wrap bare URL in a Markdown link.Apply this diff:
-4. Now you can visit http://localhost:3000/form/latest/docs/overview in the browser and see the changes you make in `tanstack/form/docs`. +4. Now you can visit [http://localhost:3000/form/latest/docs/overview](http://localhost:3000/form/latest/docs/overview) in the browser and see the changes you make in `tanstack/form/docs`.
68-72: Remove blank line inside blockquote and reduce excessive exclamation marks.Blockquotes should not have internal blank lines. Additionally, consider reducing the number of exclamation marks throughout the document for a more professional tone.
Apply this diff:
> [!NOTE] > The updated pages need to be manually reloaded in the browser. -> > [!WARNING] -> You will need to update the `docs/config.json` file (in the project's repo) if you add a new doc page! +> You will need to update the `docs/config.json` file (in the project's repo) if you add a new doc page.examples/solid/start-basic-auth/src/routes/_authed/posts.$postId.tsx (1)
6-8: Consider removing the redundant wrapper.
PostErrorComponentsimply forwards toErrorComponentwithout adding custom logic. You could useErrorComponentdirectly in the route definition.Apply this diff to simplify:
-export function PostErrorComponent({ error }: ErrorComponentProps) { - return <ErrorComponent error={error} /> -} - function PostComponent() { const post = Route.useLoaderData() @@ -20,7 +16,7 @@ export const Route = createFileRoute('/_authed/posts/$postId')({ loader: ({ params: { postId } }) => fetchPost({ data: postId }), - errorComponent: PostErrorComponent, + errorComponent: ErrorComponent, component: PostComponent, notFoundComponent: () => { return <NotFound>Post not found</NotFound>If you need custom error handling in the future, this can be revisited.
examples/solid/start-basic-auth/src/routes/signup.tsx (2)
12-63: Add input validation for email and password.The handler accepts email and password without validation. Consider adding:
- Email format validation (RFC-compliant email pattern)
- Password strength requirements (minimum length, complexity rules)
- Sanitization to prevent injection attacks
Example validation:
.handler(async ({ data }) => { // Validate email format const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/ if (!emailRegex.test(data.email)) { return { error: true, message: 'Invalid email format' } } // Validate password strength if (data.password.length < 8) { return { error: true, message: 'Password must be at least 8 characters' } } // ... rest of handler })
81-86: Include redirectUrl in the mutation payload.The server function's
inputValidatoraccepts an optionalredirectUrlparameter, but the client mutation doesn't extract or pass it. If you want to support redirection after signup (e.g., from a URL search param), you should include it here.Example with redirect support:
onSubmit={(e) => { const formData = new FormData(e.target as any as HTMLFormElement) const searchParams = new URLSearchParams(window.location.search) const redirectUrl = searchParams.get('redirect') || undefined signupMutation.mutate({ email: formData.get('email') as string, password: formData.get('password') as string, redirectUrl, }) }}examples/solid/start-basic-auth/src/routes/_authed/posts.route.tsx (2)
24-26: Active class likely overrides base classes.If Solid Router doesn’t merge class strings, activeProps.class will replace the base class. Consider duplicating essential classes in activeProps or using classList to toggle only the diff.
15-16: Clarify the “Non-existent Post” sentinel.Add a brief comment noting this is intentional to exercise the NotFound boundary, to avoid confusion for readers.
examples/solid/start-basic-auth/src/routes/__root.tsx (1)
138-138: Gate devtools to development only.Avoid shipping devtools in production bundles.
- <TanStackRouterDevtools position="bottom-right" /> + {import.meta.env.DEV ? ( + <TanStackRouterDevtools position="bottom-right" /> + ) : null}examples/solid/start-basic-auth/src/routes/_authed.tsx (2)
7-9: Add real input validation..inputValidator just echoes input. Validate email/password (min length, format) with zod/valibot.
-import { createServerFn } from '@tanstack/solid-start' +import { createServerFn } from '@tanstack/solid-start' +import { z } from 'zod' +const LoginSchema = z.object({ + email: z.string().email(), + password: z.string().min(8), +}) export const loginFn = createServerFn({ method: 'POST' }) - .inputValidator((d: { email: string; password: string }) => d) + .inputValidator((d: unknown) => LoginSchema.parse(d))
40-43: Return an explicit success payload.Helps client code avoid relying on undefined.
await session.update({ userEmail: user.email, }) + return { ok: true }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (15)
examples/solid/start-basic-auth/.gitignore(1 hunks)examples/solid/start-basic-auth/.vscode/settings.json(1 hunks)examples/solid/start-basic-auth/README.md(1 hunks)examples/solid/start-basic-auth/package.json(1 hunks)examples/solid/start-basic-auth/src/routeTree.gen.ts(1 hunks)examples/solid/start-basic-auth/src/router.tsx(1 hunks)examples/solid/start-basic-auth/src/routes/__root.tsx(1 hunks)examples/solid/start-basic-auth/src/routes/_authed.tsx(1 hunks)examples/solid/start-basic-auth/src/routes/_authed/posts.$postId.tsx(1 hunks)examples/solid/start-basic-auth/src/routes/_authed/posts.index.tsx(1 hunks)examples/solid/start-basic-auth/src/routes/_authed/posts.route.tsx(1 hunks)examples/solid/start-basic-auth/src/routes/logout.tsx(1 hunks)examples/solid/start-basic-auth/src/routes/signup.tsx(1 hunks)examples/solid/start-basic-auth/src/utils/posts.ts(1 hunks)examples/solid/start-basic-auth/vite.config.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- examples/solid/start-basic-auth/.vscode/settings.json
🚧 Files skipped from review as they are similar to previous changes (6)
- examples/solid/start-basic-auth/src/utils/posts.ts
- examples/solid/start-basic-auth/src/routes/logout.tsx
- examples/solid/start-basic-auth/vite.config.ts
- examples/solid/start-basic-auth/.gitignore
- examples/solid/start-basic-auth/src/routes/_authed/posts.index.tsx
- examples/solid/start-basic-auth/src/router.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
examples/solid/start-basic-auth/src/routes/signup.tsxexamples/solid/start-basic-auth/src/routes/_authed/posts.$postId.tsxexamples/solid/start-basic-auth/src/routes/_authed.tsxexamples/solid/start-basic-auth/src/routes/_authed/posts.route.tsxexamples/solid/start-basic-auth/src/routeTree.gen.tsexamples/solid/start-basic-auth/src/routes/__root.tsx
**/src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Place file-based routes under src/routes/ directories
Files:
examples/solid/start-basic-auth/src/routes/signup.tsxexamples/solid/start-basic-auth/src/routes/_authed/posts.$postId.tsxexamples/solid/start-basic-auth/src/routes/_authed.tsxexamples/solid/start-basic-auth/src/routes/_authed/posts.route.tsxexamples/solid/start-basic-auth/src/routes/__root.tsx
examples/{react,solid}/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep example applications under examples/react/ and examples/solid/
Files:
examples/solid/start-basic-auth/src/routes/signup.tsxexamples/solid/start-basic-auth/src/routes/_authed/posts.$postId.tsxexamples/solid/start-basic-auth/src/routes/_authed.tsxexamples/solid/start-basic-auth/src/routes/_authed/posts.route.tsxexamples/solid/start-basic-auth/src/routeTree.gen.tsexamples/solid/start-basic-auth/package.jsonexamples/solid/start-basic-auth/src/routes/__root.tsxexamples/solid/start-basic-auth/README.md
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Use workspace:* protocol for internal dependencies in package.json files
Files:
examples/solid/start-basic-auth/package.json
🧠 Learnings (1)
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
PR: TanStack/router#0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to **/package.json : Use workspace:* protocol for internal dependencies in package.json files
Applied to files:
examples/solid/start-basic-auth/package.json
🧬 Code graph analysis (6)
examples/solid/start-basic-auth/src/routes/signup.tsx (4)
examples/solid/start-basic-auth/src/utils/prisma.ts (2)
prismaClient(4-4)hashPassword(6-16)examples/solid/start-basic-auth/src/utils/session.ts (1)
useAppSession(9-13)examples/solid/start-basic-auth/src/hooks/useMutation.ts (1)
useMutation(3-41)examples/solid/start-basic-auth/src/components/Auth.tsx (1)
Auth(3-59)
examples/solid/start-basic-auth/src/routes/_authed/posts.$postId.tsx (4)
examples/solid/start-basic-auth/src/routes/_authed.tsx (1)
Route(45-58)examples/solid/start-basic-auth/src/routes/_authed/posts.route.tsx (1)
Route(4-7)examples/solid/start-basic-auth/src/utils/posts.ts (1)
fetchPost(11-27)examples/solid/start-basic-auth/src/components/NotFound.tsx (1)
NotFound(3-25)
examples/solid/start-basic-auth/src/routes/_authed.tsx (4)
examples/solid/start-basic-auth/src/utils/prisma.ts (2)
prismaClient(4-4)hashPassword(6-16)examples/solid/start-basic-auth/src/utils/session.ts (1)
useAppSession(9-13)examples/solid/start-basic-auth/src/routes/__root.tsx (1)
Route(32-87)examples/solid/start-basic-auth/src/components/Login.tsx (1)
Login(8-71)
examples/solid/start-basic-auth/src/routes/_authed/posts.route.tsx (5)
examples/solid/start-basic-auth/src/routes/__root.tsx (1)
Route(32-87)examples/solid/start-basic-auth/src/routes/_authed.tsx (1)
Route(45-58)examples/solid/start-basic-auth/src/routes/_authed/posts.$postId.tsx (1)
Route(21-28)examples/solid/start-basic-auth/src/routes/_authed/posts.index.tsx (1)
Route(3-5)examples/solid/start-basic-auth/src/utils/posts.ts (1)
fetchPosts(29-37)
examples/solid/start-basic-auth/src/routeTree.gen.ts (2)
examples/react/start-basic-auth/src/routeTree.gen.ts (6)
FileRoutesByFullPath(61-69)FileRoutesByTo(70-77)FileRoutesById(78-88)FileRouteTypes(89-112)RootRouteChildren(113-119)routeTree(213-215)examples/solid/start-basic-auth/src/router.tsx (1)
getRouter(6-16)
examples/solid/start-basic-auth/src/routes/__root.tsx (6)
examples/solid/start-basic-auth/src/utils/session.ts (1)
useAppSession(9-13)examples/solid/start-basic-auth/src/routes/_authed.tsx (1)
Route(45-58)examples/solid/start-basic-auth/src/routes/logout.tsx (1)
Route(15-18)examples/solid/start-basic-auth/src/utils/seo.ts (1)
seo(1-33)examples/solid/start-basic-auth/src/components/DefaultCatchBoundary.tsx (1)
DefaultCatchBoundary(10-53)examples/solid/start-basic-auth/src/components/NotFound.tsx (1)
NotFound(3-25)
🪛 LanguageTool
examples/solid/start-basic-auth/README.md
[uncategorized] ~25-~25: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ...TanStack Router app. In production, the markdown doc pages are fetched from the GitHub r...
(MARKDOWN_NNP)
[style] ~71-~71: Using many exclamation marks might seem excessive (in this case: 8 exclamation marks for a text that’s 1725 characters long)
Context: ...e manually reloaded in the browser. > [!WARNING] > You will need to update the docs/config.json file (in the project's repo) if you add a new doc page!
(EN_EXCESSIVE_EXCLAMATION)
🪛 markdownlint-cli2 (0.18.1)
examples/solid/start-basic-auth/README.md
46-46: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
53-53: Blank line inside blockquote
(MD028, no-blanks-blockquote)
66-66: Bare URL used
(MD034, no-bare-urls)
70-70: Blank line inside blockquote
(MD028, no-blanks-blockquote)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Preview
- GitHub Check: Test
🔇 Additional comments (3)
examples/solid/start-basic-auth/src/routes/_authed/posts.$postId.tsx (2)
1-4: LGTM!Imports are clean and correctly structured.
10-19: LGTM!Correctly uses SolidJS signal pattern by calling
post()as an accessor function. The component structure is clean and appropriate.examples/solid/start-basic-auth/src/routeTree.gen.ts (1)
1-224: Generated file — no review required.Looks consistent with routes; no action.
| # Welcome to TanStack.com! | ||
|
|
||
| This site is built with TanStack Router! | ||
|
|
||
| - [TanStack Router Docs](https://tanstack.com/router) | ||
|
|
||
| It's deployed automagically with Netlify! | ||
|
|
||
| - [Netlify](https://netlify.com/) | ||
|
|
||
| ## Development | ||
|
|
||
| From your terminal: | ||
|
|
||
| ```sh | ||
| pnpm install | ||
| pnpm dev | ||
| ``` | ||
|
|
||
| This starts your app in development mode, rebuilding assets on file changes. | ||
|
|
||
| ## Editing and previewing the docs of TanStack projects locally | ||
|
|
||
| The documentations for all TanStack projects except for `React Charts` are hosted on [https://tanstack.com](https://tanstack.com), powered by this TanStack Router app. | ||
| In production, the markdown doc pages are fetched from the GitHub repos of the projects, but in development they are read from the local file system. | ||
|
|
||
| Follow these steps if you want to edit the doc pages of a project (in these steps we'll assume it's [`TanStack/form`](https://github.com/tanstack/form)) and preview them locally : | ||
|
|
||
| 1. Create a new directory called `tanstack`. | ||
|
|
||
| ```sh | ||
| mkdir tanstack | ||
| ``` | ||
|
|
||
| 2. Enter the directory and clone this repo and the repo of the project there. | ||
|
|
||
| ```sh | ||
| cd tanstack | ||
| git clone git@github.com:TanStack/tanstack.com.git | ||
| git clone git@github.com:TanStack/form.git | ||
| ``` | ||
|
|
||
| > [!NOTE] | ||
| > Your `tanstack` directory should look like this: | ||
| > | ||
| > ``` | ||
| > tanstack/ | ||
| > | | ||
| > +-- form/ | ||
| > | | ||
| > +-- tanstack.com/ | ||
| > ``` | ||
|
|
||
| > [!WARNING] | ||
| > Make sure the name of the directory in your local file system matches the name of the project's repo. For example, `tanstack/form` must be cloned into `form` (this is the default) instead of `some-other-name`, because that way, the doc pages won't be found. | ||
|
|
||
| 3. Enter the `tanstack/tanstack.com` directory, install the dependencies and run the app in dev mode: | ||
|
|
||
| ```sh | ||
| cd tanstack.com | ||
| pnpm i | ||
| # The app will run on https://localhost:3000 by default | ||
| pnpm dev | ||
| ``` | ||
|
|
||
| 4. Now you can visit http://localhost:3000/form/latest/docs/overview in the browser and see the changes you make in `tanstack/form/docs`. | ||
|
|
||
| > [!NOTE] | ||
| > The updated pages need to be manually reloaded in the browser. | ||
|
|
||
| > [!WARNING] | ||
| > You will need to update the `docs/config.json` file (in the project's repo) if you add a new doc page! |
There was a problem hiding this comment.
Rewrite the README to document the basic-auth example application.
The current README content describes the TanStack.com local development workflow, but this file should document the Solid Start basic-auth example application. Per the PR objectives and AI summary, this example demonstrates server-side authentication with login/signup, session management, protected routes, and Prisma SQLite integration.
The README should explain:
- What this example demonstrates (basic authentication patterns with Solid Start)
- How to run the example locally
- Project structure and key features
- Database setup and schema
- Authentication workflows and protected route guards
- Usage patterns or key code snippets
Would you like me to generate a corrected README that documents the basic-auth example application based on the structure described in the broader PR?
🧰 Tools
🪛 LanguageTool
[uncategorized] ~25-~25: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ...TanStack Router app. In production, the markdown doc pages are fetched from the GitHub r...
(MARKDOWN_NNP)
[style] ~71-~71: Using many exclamation marks might seem excessive (in this case: 8 exclamation marks for a text that’s 1725 characters long)
Context: ...e manually reloaded in the browser. > [!WARNING] > You will need to update the docs/config.json file (in the project's repo) if you add a new doc page!
(EN_EXCESSIVE_EXCLAMATION)
🪛 markdownlint-cli2 (0.18.1)
46-46: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
53-53: Blank line inside blockquote
(MD028, no-blanks-blockquote)
66-66: Bare URL used
(MD034, no-bare-urls)
70-70: Blank line inside blockquote
(MD028, no-blanks-blockquote)
🤖 Prompt for AI Agents
In examples/solid/start-basic-auth/README.md lines 1-72, the README currently
contains unrelated TanStack.com developer docs; replace it with documentation
for the Solid Start basic-auth example: summarize what the example demonstrates
(server-side auth, login/signup, session management, protected routes,
Prisma+SQLite), provide concise steps to run locally (install, dev, env and DB
setup), outline project structure and key files, describe the database schema
and how to initialize/migrate the SQLite DB, and explain authentication flows
and route guards with pointers to the key code locations; ensure the new README
is focused, actionable, and contains no unrelated TanStack.com instructions.
| // Check if the user exists | ||
| if (!user) { | ||
| return { | ||
| error: true, | ||
| userNotFound: true, | ||
| message: 'User not found', | ||
| } | ||
| } |
There was a problem hiding this comment.
Avoid user enumeration in error responses.
Returning “User not found” reveals account existence. Use a generic message for both cases.
- if (!user) {
- return { error: true, userNotFound: true, message: 'User not found' }
- }
+ if (!user) {
+ // Generic failure to prevent user enumeration
+ return { error: true, message: 'Invalid email or password' }
+ }Also align the mismatch branch to the same message.
- return { error: true, message: 'Incorrect password' }
+ return { error: true, message: 'Invalid email or password' }📝 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.
| // Check if the user exists | |
| if (!user) { | |
| return { | |
| error: true, | |
| userNotFound: true, | |
| message: 'User not found', | |
| } | |
| } | |
| // Check if the user exists | |
| if (!user) { | |
| // Generic failure to prevent user enumeration | |
| return { error: true, message: 'Invalid email or password' } | |
| } |
🤖 Prompt for AI Agents
In examples/solid/start-basic-auth/src/routes/_authed.tsx around lines 17 to 24,
the code returns a specific "User not found" message which allows user
enumeration; change this to a generic authentication failure message and ensure
the password-mismatch branch returns the exact same generic response shape and
message. Replace the detailed userNotFound flag/message with a single generic
error payload (e.g., error: true, authFailed: true, message: 'Invalid username
or password') and apply that same payload in the branch that handles password
mismatch so both branches are indistinguishable.
| const hashedPassword = await hashPassword(data.password) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Password hashing design is weak (static salt).
hashPassword (utils/prisma.ts) uses a fixed salt. Use a modern password hasher (argon2id/bcrypt) with per‑password salt and params, and update signup/login accordingly.
Based on best practices
🤖 Prompt for AI Agents
In examples/solid/start-basic-auth/src/routes/_authed.tsx around lines 27-28,
the code calls hashPassword which currently uses a static salt; replace this by
switching the hashing implementation to a modern adaptive hasher (e.g. argon2id
or bcrypt) that generates a unique per-password salt and encodes
algorithm/params in the stored hash (so verification is stateless). Update
utils/prisma.ts: remove static salt, use an argon2id/bcrypt library to hash with
secure defaults (memory/time/threads or cost) and return the full encoded hash;
update signup to store that encoded hash and update login to use the library's
verify function against the stored hash. Ensure migrations/DB column length
supports full encoded hash and add minimal error handling around
hashing/verification.
| export const Route = createFileRoute('/_authed/posts/$postId')({ | ||
| loader: ({ params: { postId } }) => fetchPost({ data: postId }), | ||
| errorComponent: PostErrorComponent, | ||
| component: PostComponent, | ||
| notFoundComponent: () => { | ||
| return <NotFound>Post not found</NotFound> | ||
| }, | ||
| }) |
There was a problem hiding this comment.
Fix the loader invocation to match the server function signature.
Line 22 incorrectly calls fetchPost({ data: postId }), but based on the fetchPost definition in examples/solid/start-basic-auth/src/utils/posts.ts, the inputValidator expects a string directly, not an object. The server function should be called as fetchPost(postId).
Apply this diff to fix the loader:
export const Route = createFileRoute('/_authed/posts/$postId')({
- loader: ({ params: { postId } }) => fetchPost({ data: postId }),
+ loader: ({ params: { postId } }) => fetchPost(postId),
errorComponent: PostErrorComponent,
component: PostComponent,
notFoundComponent: () => {📝 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.
| export const Route = createFileRoute('/_authed/posts/$postId')({ | |
| loader: ({ params: { postId } }) => fetchPost({ data: postId }), | |
| errorComponent: PostErrorComponent, | |
| component: PostComponent, | |
| notFoundComponent: () => { | |
| return <NotFound>Post not found</NotFound> | |
| }, | |
| }) | |
| export const Route = createFileRoute('/_authed/posts/$postId')({ | |
| loader: ({ params: { postId } }) => fetchPost(postId), | |
| errorComponent: PostErrorComponent, | |
| component: PostComponent, | |
| notFoundComponent: () => { | |
| return <NotFound>Post not found</NotFound> | |
| }, | |
| }) |
🤖 Prompt for AI Agents
In examples/solid/start-basic-auth/src/routes/_authed/posts.$postId.tsx around
lines 21 to 28, the loader currently calls fetchPost({ data: postId }) but
fetchPost expects the postId string directly; update the loader to call
fetchPost(postId) so the input matches the server function's inputValidator.
| params={{ | ||
| postId: post.id, | ||
| }} |
There was a problem hiding this comment.
Fix param type: ensure postId is a string.
Route '/posts/$postId' expects a string param. JSONPlaceholder ids are numbers; passing a number breaks strict typing.
- params={{
- postId: post.id,
- }}
+ params={{
+ postId: String(post.id),
+ }}📝 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.
| params={{ | |
| postId: post.id, | |
| }} | |
| params={{ | |
| postId: String(post.id), | |
| }} |
🤖 Prompt for AI Agents
In examples/solid/start-basic-auth/src/routes/_authed/posts.route.tsx around
lines 21 to 23, the route param postId is passed as a number (JSONPlaceholder
id) but the route expects a string; convert the id to a string before passing it
(e.g., use String(post.id) or post.id.toString()), ensuring null/undefined
safety if necessary so the params.postId is always a string.
| // Encrypt the password using Sha256 into plaintext | ||
| const password = await hashPassword(data.password) |
There was a problem hiding this comment.
Security: hardcoded salt in password hashing.
Based on the relevant code snippet, hashPassword uses a hardcoded salt value 'salt'. This is a critical security vulnerability as the same salt for all passwords makes rainbow table attacks feasible and defeats the purpose of salting.
Each user should have a unique, randomly generated salt stored alongside their password hash in the database. The hashPassword function should accept both password and salt parameters, and salt generation should happen before user creation.
🤖 Prompt for AI Agents
In examples/solid/start-basic-auth/src/routes/signup.tsx around lines 20-21, the
code calls hashPassword(data.password) which uses a hardcoded salt; change flow
to generate a unique, cryptographically secure random salt for each user before
hashing, update hashPassword to accept (password, salt) (or switch to a secure
library like bcrypt/argon2 that handles per-user salts internally), call it with
the generated salt, and persist the salt alongside the resulting password hash
in the user record; ensure salt generation uses a secure RNG and the hashing
routine uses a standard slow algorithm (bcrypt/argon2/PBKDF2) with appropriate
work factor.
Correct the misleading comment.
The comment states "Encrypt the password using Sha256 into plaintext" which is incorrect. The function is hashing (not encrypting) the password using PBKDF2 with SHA-256, producing a hex-encoded string (not plaintext).
Apply this diff:
- // Encrypt the password using Sha256 into plaintext
+ // Hash the password using PBKDF2 with SHA-256
const password = await hashPassword(data.password)📝 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.
| // Encrypt the password using Sha256 into plaintext | |
| const password = await hashPassword(data.password) | |
| // Hash the password using PBKDF2 with SHA-256 | |
| const password = await hashPassword(data.password) |
🤖 Prompt for AI Agents
In examples/solid/start-basic-auth/src/routes/signup.tsx around lines 20 to 21,
the comment "Encrypt the password using Sha256 into plaintext" is misleading;
update it to accurately state that the function is hashing the password using
PBKDF2 with SHA-256 and returning a hex-encoded string. Replace the comment with
a concise line such as: "Hash the password using PBKDF2 with SHA-256 and return
a hex-encoded string" (do not say encrypt or plaintext).
| if (found) { | ||
| if (found.password !== password) { | ||
| return { | ||
| error: true, | ||
| userExists: true, | ||
| message: 'User already exists', | ||
| } | ||
| } | ||
|
|
||
| // Store the user's email in the session | ||
| await session.update({ | ||
| userEmail: found.email, | ||
| }) | ||
|
|
||
| // Redirect to the prev page stored in the "redirect" search param | ||
| throw redirect({ | ||
| href: data.redirectUrl || '/', | ||
| }) | ||
| } |
There was a problem hiding this comment.
Reconsider the login-on-signup behavior.
Lines 26-44 implement unusual logic where if a user already exists during signup and their password matches, the system logs them in instead of rejecting the signup attempt. Typical authentication flows reject signup attempts for existing users regardless of password match, as this conflates signup and login concerns and may confuse users.
Consider returning an error for all existing users:
if (found) {
- if (found.password !== password) {
- return {
- error: true,
- userExists: true,
- message: 'User already exists',
- }
- }
-
- // Store the user's email in the session
- await session.update({
- userEmail: found.email,
- })
-
- // Redirect to the prev page stored in the "redirect" search param
- throw redirect({
- href: data.redirectUrl || '/',
- })
+ return {
+ error: true,
+ userExists: true,
+ message: 'User already exists',
+ }
}🤖 Prompt for AI Agents
In examples/solid/start-basic-auth/src/routes/signup.tsx around lines 26 to 44,
the current logic logs an existing user in when the provided password matches;
instead, always treat an existing account as a signup conflict and return an
error. Remove the session.update and redirect code inside the found branch and
replace it with a consistent error response indicating the user already exists
and prompting them to log in (do not authenticate or start a session in the
signup flow for existing users).
Summary by CodeRabbit
Release Notes