Conversation
- Changed PostCSS configuration to use "@tailwindcss/postcss" instead of "tailwindcss". - Updated revalidation calls in various files to include a default argument for cache tags. - Refactored global CSS to replace Tailwind directives with an import statement and theme variables. - Adjusted tests to reflect changes in revalidation calls and mock implementations. - Added a script to automate the CSS fix for Tailwind directives.
There was a problem hiding this comment.
Pull request overview
This PR modernizes the Narravo Next.js app by upgrading core dependencies (Next/Tailwind/Vitest/etc.), migrating the Tailwind/PostCSS setup, improving file-handling performance via streaming, and hardening configuration reads with sensible defaults.
Changes:
- Upgraded major dependencies and migrated PostCSS/Tailwind configuration for Tailwind v4+.
- Refactored export/download and uploads serving to use streaming and stat-based sizing/checksums.
- Added config fallbacks and updated tests/mocks to match new behaviors and signatures.
Reviewed changes
Copilot reviewed 51 out of 53 changed files in this pull request and generated 19 comments.
Show a summary per file
| File | Description |
|---|---|
| vitest.config.ts | Removes explicit worker pool configuration from Vitest. |
| vite.config.mts | Removes environmentMatchGlobs (environment now set per-test). |
| tsconfig.json | Switches JSX transform to react-jsx and adjusts includes/formatting. |
| tests/upload-tracking.test.ts | Adds user/post fixtures for upload tracking tests. |
| tests/tiptap-editor.test.tsx | Adds JSDoc-style jsdom environment directive. |
| tests/scripts/seed-config.test.ts | Updates mocking style for ConfigServiceImpl. |
| tests/s3.test.ts | Updates AWS SDK client mock construction style. |
| tests/rss-icon.test.tsx | Adds JSDoc-style jsdom environment directive. |
| tests/revalidation.test.ts | Updates expectations for new revalidateTag call signature. |
| tests/rateLimit.test.ts | Updates config mock construction style. |
| tests/proxy.test.ts | Updates tests to target proxy handler instead of middleware. |
| tests/page-view-tracking.test.ts | Updates config mock construction style. |
| tests/page-view-api.test.ts | Updates config mock construction style. |
| tests/moderation.test.ts | Updates expectations for new revalidateTag call signature. |
| tests/local-storage.test.ts | Updates mocks to match new stat-based size usage. |
| tests/db.test.ts | Clears new DB globals between tests; updates pool constructor mocks. |
| tests/data-operations.test.ts | Updates fs mocks to support streaming export/download behavior. |
| tests/components/auth/UserMenu.test.tsx | Adds JSDoc-style jsdom environment directive. |
| tests/components/auth/TwoFactorVerification.test.tsx | Adds JSDoc-style jsdom environment directive. |
| tests/components/auth/TwoFactorGuard.test.tsx | Adds JSDoc-style jsdom environment directive; adjusts session mock typing. |
| tests/comment-form-timing.test.ts | Updates config mock construction style. |
| tests/comment-auto-approval.test.ts | Updates config mock construction style and mock implementations. |
| tests/api/uploads-local-api.test.ts | Adjusts auth/logger mocks and uses unique upload key in test. |
| tests/api/uploads-banner-api.test.ts | Adjusts auth mock; changes UUID generation expectation. |
| tests/api/r2-sign-api.test.ts | Updates config/S3 mocks; adds additional logger methods in mock. |
| tests/api/metrics-view-api.test.ts | Updates config mock construction style. |
| tests/api/admin-data-ops.test.ts | Updates fs mocks for streaming export/download; stat-based size. |
| tests/api/admin-config.test.ts | Updates config mock construction style. |
| tests/analytics-view-tracking.test.ts | Updates config mock construction style. |
| tests/actions/deletePost.test.ts | Updates expectations for new revalidateTag call signature. |
| tests/2fa-totp.test.ts | Switches TOTP import to @otplib/v12-adapter. |
| src/proxy.ts | Renames exported handler from middleware to proxy and retains matcher config. |
| src/lib/revalidation.ts | Updates revalidateTag calls to include a new second argument. |
| src/lib/reactions.ts | Updates reaction cache revalidation calls to include a new second argument. |
| src/lib/moderation.ts | Updates moderation cache revalidation calls to include a new second argument. |
| src/lib/db.ts | Adds global caching for pg pool/query patching outside production. |
| src/lib/2fa/totp.ts | Switches TOTP import to @otplib/v12-adapter. |
| src/components/comments/actions.ts | Adds default fallbacks for comment config; updates tag revalidation call. |
| src/components/auth/UserMenu.tsx | Improves text contrast by adding explicit foreground classes. |
| src/components/Sidebar.tsx | Adds default fallback values for sidebar config. |
| src/app/uploads/[...path]/route.ts | Serves uploads via streaming instead of readFileSync. |
| src/app/page.tsx | Adds default fallback values for homepage config reads. |
| src/app/globals.css | Migrates Tailwind directives to @import "tailwindcss" + @theme variables. |
| src/app/api/r2/sign/route.ts | Adds fallback limits for uploads configuration. |
| src/app/api/admin/export/route.ts | Streams checksum calculation and uses stat size instead of buffer length. |
| src/app/api/admin/export/[operationId]/route.ts | Streams export downloads and uses stat-based Content-Length. |
| src/app/actions/deletePost.ts | Updates cache revalidation calls to include a new second argument. |
| src/app/(admin)/admin/posts/actions.ts | Updates cache revalidation calls to include a new second argument. |
| postcss.config.cjs | Switches to @tailwindcss/postcss plugin. |
| package.json | Upgrades multiple dependencies; adds @tailwindcss/postcss and @otplib/v12-adapter. |
| middleware.ts | Deletes the Next.js middleware entrypoint file. |
| fix-css.mjs | Adds a one-off script to rewrite Tailwind directives in globals.css. |
Comments suppressed due to low confidence (1)
src/proxy.ts:22
- Next.js only executes middleware from a
middleware.tsfile (root orsrc/middleware.ts) and expects an exportedmiddlewarefunction. Renaming the handler toproxyinsrc/proxy.ts(and deletingmiddleware.ts) means redirects will no longer run in production; consider restoring amiddleware.tsthat exportsmiddleware, or move/rename this file accordingly.
|
|
||
| // Revalidate the post page to show new comment (when approved) | ||
| revalidateTag(`post:${params.postId}`); | ||
| revalidateTag(`post:${params.postId}`, "default" as any); |
There was a problem hiding this comment.
This revalidateTag(..., "default" as any) call suppresses type checking in a server action path. Please update to the correct revalidateTag signature (or call via a typed wrapper) so the code compiles without as any.
| revalidateTag(`post:${params.postId}`, "default" as any); | |
| revalidateTag(`post:${params.postId}`); |
| revalidateTag("home", "default" as any); // Home page cache | ||
| revalidateTag(`post:${id}`, "default" as any); // Post-specific cache |
There was a problem hiding this comment.
revalidateTag("home", "default" as any) relies on an as any cast to satisfy types, which weakens the strict TS guarantees in this codebase. Prefer calling revalidateTag using its actual typed signature (or via a typed wrapper) so this compiles without casts.
| revalidateTag("home", "default" as any); // Home page cache | |
| revalidateTag(`post:${id}`, "default" as any); // Post-specific cache | |
| revalidateTag("home"); // Home page cache | |
| revalidateTag(`post:${id}`); // Post-specific cache |
| /** @vitest-environment jsdom */ | ||
|
|
||
| // SPDX-License-Identifier: Apache-2.0 | ||
| // @vitest-environment jsdom | ||
| import "@testing-library/jest-dom/vitest"; |
There was a problem hiding this comment.
This file has duplicated Vitest environment directives (/** @vitest-environment jsdom */ and // @vitest-environment jsdom). Please remove one of them and standardize on a single directive style.
| /** @vitest-environment jsdom */ | ||
|
|
||
| // SPDX-License-Identifier: Apache-2.0 | ||
| // @vitest-environment jsdom | ||
| import "@testing-library/jest-dom/vitest"; |
There was a problem hiding this comment.
This file now contains two Vitest environment directives (/** @vitest-environment jsdom */ and // @vitest-environment jsdom). Please keep only one to reduce noise and avoid confusion.
| // Mock session data | ||
| let mockSessionData: ReturnType<typeof vi.fn> | null = null; | ||
| let mockSessionData: any = null; | ||
| let mockSessionStatus: "loading" | "authenticated" | "unauthenticated" = "loading"; |
There was a problem hiding this comment.
mockSessionData was widened to any, which weakens type checking in this test. Prefer a narrower type (e.g., a function type like (() => Session | null) | null, or ReturnType<typeof vi.fn> | null) so accidental misuse is caught by TypeScript.
| import fs from 'fs'; | ||
| const file = 'src/app/globals.css'; | ||
| let content = fs.readFileSync(file, 'utf8'); |
There was a problem hiding this comment.
This new script is missing the SPDX license header that other repo scripts include (e.g., scripts/write-version.mjs:1, scripts/audit-admin-2fa.mjs:2). Add // SPDX-License-Identifier: Apache-2.0 at the top (and consider moving this into scripts/ or removing it if it was only a one-off migration helper).
| /** @vitest-environment jsdom */ | ||
|
|
||
| // SPDX-License-Identifier: Apache-2.0 | ||
| // @vitest-environment jsdom | ||
| import React from "react"; |
There was a problem hiding this comment.
This file now has two Vitest environment directives (/** @vitest-environment jsdom */ and // @vitest-environment jsdom). Keeping both is redundant and can be confusing; please remove one and standardize on a single form.
| /** @vitest-environment jsdom */ | ||
|
|
||
| // SPDX-License-Identifier: Apache-2.0 | ||
| // @vitest-environment jsdom | ||
| import "@testing-library/jest-dom/vitest"; |
There was a problem hiding this comment.
This test file now declares the Vitest environment twice (/** @vitest-environment jsdom */ and // @vitest-environment jsdom). Drop one so the environment configuration is unambiguous and consistent.
| beforeAll(async () => { | ||
| // Create test user | ||
| await db.insert(users).values({ | ||
| id: testUserId, | ||
| email: 'upload-tracking-test@example.com', | ||
| name: 'Upload Test User', | ||
| }); |
There was a problem hiding this comment.
beforeAll inserts fixed users/posts IDs without handling the case where they already exist (e.g., if a previous run crashed before afterAll, or tests run against a shared DB). Consider deleting any existing rows first or using an upsert/on-conflict strategy to make the test idempotent.
| revalidateTag(`post:${targetId}`, "default" as any); | ||
| revalidateTag("home", "default" as any); |
There was a problem hiding this comment.
These revalidateTag(..., "default" as any) calls are suppressing TypeScript errors rather than using a properly typed API. Please align with the real revalidateTag signature (or add a typed wrapper) so this doesn't rely on as any.
This pull request introduces several improvements and updates across the codebase, focusing on dependency upgrades, performance optimizations for file handling, configuration fallback enhancements, and a migration to the new Tailwind/PostCSS setup. The most significant changes are grouped below.
1. Dependency and Build System Upgrades
package.json, including major updates tonext,@tiptap/*,@aws-sdk/*,tailwindcss,autoprefixer, and others. Added new dependencies such as@otplib/v12-adapterand updated dev dependencies for improved compatibility and security.@tailwindcss/postcssplugin, aligning with Tailwind CSS v4+ requirements.2. Tailwind CSS Migration
@tailwinddirectives inglobals.csswith@import "tailwindcss"and introduced a@themeblock for CSS variables, reflecting Tailwind v4+ migration best practices. [1] [2]3. Performance and File Handling Improvements
fs.createReadStreamand Node.js streams) instead of loading entire files into memory, improving performance and scalability for large files in API routes and uploads. (src/app/api/admin/export/[operationId]/route.tsL46-R61, src/app/api/admin/export/route.tsL46-R56, src/app/uploads/[...path]/route.tsR4, src/app/uploads/[...path]/route.tsL47-R53)4. Configuration and Default Fallbacks
5. UI and Minor Fixes
UserMenucomponent for better accessibility and theme consistency. [1] [2]These changes collectively modernize the project’s dependencies and build system, improve performance for file operations, enhance configuration resilience, and ensure compatibility with the latest Tailwind CSS ecosystem.