🎨 Add linting and prettier to more places #1432
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Caution Review failedThe pull request is closed. WalkthroughThe pull request introduces various minor formatting changes across multiple files, primarily focusing on import statements and class name adjustments for consistency and readability. Key functionalities, such as locale handling in middleware and user interface components, remain unchanged. Additionally, new components are added in the marketing section, while several scripts in package configurations are updated or removed. Overall, the changes are cosmetic, enhancing code clarity without altering core logic or functionality. Changes
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (8)
packages/posthog/package.json (1)
10-14: Consider enhancing the script configurations.The addition of linting, type checking, and formatting scripts is a good practice. However, consider these improvements:
- Make the lint script more specific and strict:
- Scope the prettier command to specific file types:
"scripts": { - "lint": "eslint ./src", + "lint": "eslint './src/**/*.{js,jsx,ts,tsx}' --max-warnings=0", "type-check": "tsc --noEmit", - "prettier": "prettier --write ." + "prettier": "prettier --write './src/**/*.{js,jsx,ts,tsx,json,css,md}'" }These changes will:
- Explicitly target relevant file extensions for linting
- Enforce zero warnings policy
- Scope prettier to specific file types and directories to avoid formatting unwanted files
packages/billing/package.json (1)
13-15: LGTM! Consider adding --fix flag to lint script.The addition of prettier and lint scripts aligns well with the PR objectives. The trailing comma in the type-check script improves git diff readability.
Consider adding the
--fixflag to automatically fix auto-fixable issues:- "lint": "eslint ./src" + "lint": "eslint ./src --fix"packages/database/package.json (1)
11-12: Consider adding a lint script for consistencyOther packages in the monorepo (e.g., @rallly/billing, @rallly/posthog) include both
lintandprettierscripts. For consistency across packages, consider adding a lint script here as well.Add the lint script:
"scripts": { "db:generate": "prisma generate", "db:push": "prisma db push --skip-generate", "db:deploy": "prisma migrate deploy", "db:migrate": "prisma migrate dev", "db:seed": "tsx prisma/seed.ts", "type-check": "tsc --pretty --noEmit", + "lint": "eslint .", "prettier": "prettier --write ." },apps/web/declarations/next-auth.d.ts (1)
Line range hint
13-36: Consider extracting common properties into a shared interface.The same set of properties (locale, timeZone, timeFormat, weekStart) are repeated across all three interfaces. Consider extracting these into a shared interface to improve maintainability and reduce duplication.
interface UserPreferences { locale?: string | null; timeZone?: string | null; timeFormat?: TimeFormat | null; weekStart?: number | null; } declare module "next-auth" { interface Session { user: UserPreferences & DefaultSession["user"] & { id: string; }; } interface User extends DefaultUser, UserPreferences {} } declare module "next-auth/jwt" { interface JWT extends DefaultJWT, UserPreferences {} }apps/web/src/app/api/storage/[...key]/route.ts (1)
Line range hint
39-60: Consider enhancing error handling and security.While not directly related to the formatting changes, here are some suggestions to improve the code:
- Add specific error handling for different S3 errors (NoSuchKey, AccessDenied, etc.)
- Add validation for the imageKey to prevent path traversal
- Implement a whitelist for allowed content types
Here's a suggested implementation:
export async function GET( _req: NextRequest, context: { params: { key: string[] } }, ) { const imageKey = context.params.key.join("/"); if (!imageKey) { return new NextResponse("No key provided", { status: 400 }); } + // Prevent path traversal + if (imageKey.includes('..')) { + return new NextResponse("Invalid key", { status: 400 }); + } try { const { buffer, contentType } = await getAvatar(imageKey); + // Whitelist allowed content types + const allowedTypes = ['image/jpeg', 'image/png', 'image/gif']; + const safeContentType = allowedTypes.includes(contentType) + ? contentType + : 'application/octet-stream'; + return new NextResponse(buffer, { headers: { - "Content-Type": contentType, + "Content-Type": safeContentType, "Cache-Control": "public, max-age=3600", }, }); } catch (error) { + if (error instanceof Error) { + if (error.message === "Object not found") { + return NextResponse.json( + { error: "File not found" }, + { status: 404 } + ); + } + } console.error(error); return NextResponse.json( { error: "Failed to fetch object" }, { status: 500 }, ); } }packages/emails/src/templates/new-comment.tsx (1)
Line range hint
12-45: Consider using Trans component for preview text consistency.While the component implementation looks good overall, consider using the Trans component for the preview text to maintain consistency with other translated content in the component.
Here's a suggested improvement:
preview={ctx.t("newComment_preview", { ns: "emails", defaultValue: "Go to your poll to see what they said.", - })} + })} + // Alternative: Use Trans component for consistency + // preview={ + // <Trans + // i18n={ctx.i18n} + // ns="emails" + // i18nKey="newComment_preview" + // defaults="Go to your poll to see what they said." + // /> + // }packages/ui/src/avatar.tsx (2)
Line range hint
39-77: Consider enhancing the color system implementation.While the current implementation is functional, here are some suggestions to make it more robust:
+type ColorPair = { + bg: string; + text: string; +}; -const colorPairs = [ +const colorPairs: ColorPair[] = [ { bg: "#E6F4FF", text: "#0065BD" }, // ... other pairs ]; +const colorCache = new Map<string, { bgColor: string; textColor: string }>(); export function getColor(seed: string): { bgColor: string; textColor: string; } { + const cached = colorCache.get(seed); + if (cached) return cached; + let hash = 0; for (let i = 0; i < seed.length; i++) { hash = seed.charCodeAt(i) + ((hash << 5) - hash); } const colorPair = colorPairs[Math.abs(hash) % colorPairs.length]; - return { bgColor: colorPair.bg, textColor: colorPair.text }; + const result = { bgColor: colorPair.bg, textColor: colorPair.text }; + colorCache.set(seed, result); + return result; }These changes would:
- Add type safety for color pairs
- Implement memoization to improve performance for repeated seeds
- Make the code more maintainable
Line range hint
39-53: Consider extracting the color system to a shared utility.The color pairs and color generation logic could be useful for other components beyond avatars. Consider moving this to a shared utility module (e.g.,
packages/ui/src/utils/colors.ts) to promote reusability and maintain a single source of truth for your design system's color palette.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (36)
apps/landing/src/middleware.ts(1 hunks)apps/web/declarations/next-auth.d.ts(1 hunks)apps/web/sentry.server.config.ts(0 hunks)apps/web/src/app/[locale]/(admin)/settings/profile/delete-account-dialog.tsx(1 hunks)apps/web/src/app/[locale]/poll/[urlId]/duplicate-dialog.tsx(1 hunks)apps/web/src/app/[locale]/poll/[urlId]/edit-details/page.tsx(1 hunks)apps/web/src/app/[locale]/poll/[urlId]/edit-settings/page.tsx(1 hunks)apps/web/src/app/api/logout/route.ts(1 hunks)apps/web/src/app/api/storage/[...key]/route.ts(1 hunks)apps/web/src/app/global-error.tsx(2 hunks)apps/web/src/components/add-to-calendar-button.tsx(1 hunks)apps/web/src/components/create-poll.tsx(1 hunks)apps/web/src/components/participant-dropdown.tsx(1 hunks)apps/web/src/components/pay-wall-dialog.tsx(1 hunks)apps/web/src/components/poll-context.tsx(1 hunks)apps/web/src/components/poll/manage-poll/finalize-poll-dialog.tsx(1 hunks)apps/web/tests/create-delete-poll.spec.ts(1 hunks)apps/web/tests/edit-options.spec.ts(1 hunks)packages/billing/.eslintrc.js(1 hunks)packages/billing/package.json(1 hunks)packages/database/package.json(1 hunks)packages/emails/.eslintrc.js(1 hunks)packages/emails/package.json(1 hunks)packages/emails/src/components/styled-components.tsx(1 hunks)packages/emails/src/templates/new-comment.tsx(1 hunks)packages/posthog/package.json(1 hunks)packages/posthog/src/next/middleware.ts(1 hunks)packages/ui/package.json(1 hunks)packages/ui/src/avatar.tsx(1 hunks)packages/ui/src/flex.tsx(1 hunks)packages/ui/src/form.tsx(1 hunks)packages/ui/src/hooks/use-toast.ts(1 hunks)packages/ui/src/icon.tsx(1 hunks)packages/ui/src/toast.tsx(1 hunks)packages/utils/.eslintrc.js(1 hunks)packages/utils/package.json(1 hunks)
💤 Files with no reviewable changes (1)
- apps/web/sentry.server.config.ts
✅ Files skipped from review due to trivial changes (21)
- apps/landing/src/middleware.ts
- apps/web/src/app/[locale]/(admin)/settings/profile/delete-account-dialog.tsx
- apps/web/src/app/[locale]/poll/[urlId]/duplicate-dialog.tsx
- apps/web/src/app/[locale]/poll/[urlId]/edit-details/page.tsx
- apps/web/src/app/api/logout/route.ts
- apps/web/src/components/create-poll.tsx
- apps/web/src/components/participant-dropdown.tsx
- apps/web/src/components/pay-wall-dialog.tsx
- apps/web/src/components/poll/manage-poll/finalize-poll-dialog.tsx
- apps/web/tests/create-delete-poll.spec.ts
- apps/web/tests/edit-options.spec.ts
- packages/billing/.eslintrc.js
- packages/emails/.eslintrc.js
- packages/emails/src/components/styled-components.tsx
- packages/posthog/src/next/middleware.ts
- packages/ui/src/flex.tsx
- packages/ui/src/form.tsx
- packages/ui/src/hooks/use-toast.ts
- packages/ui/src/icon.tsx
- packages/ui/src/toast.tsx
- packages/utils/.eslintrc.js
🔇 Additional comments (15)
packages/utils/package.json (2)
6-9: LGTM! Development workflow improvements added.
Good additions that align with the PR objectives:
- Prettier formatting with
--write . - ESLint for static analysis
- TypeScript type checking
Let's verify the configuration files are in place:
#!/bin/bash
# Check for required config files
echo "Checking for Prettier config..."
fd -t f -e json -e yaml -e js "prettier"
echo "Checking for ESLint config..."
fd -t f -e json -e yaml -e js "eslint"
echo "Checking for TypeScript config..."
fd -t f "tsconfig.json"1-5: Verify the impact of removing "type": "module"
The removal of the module type declaration could affect how Node.js interprets this package and its imports. This change might require updates to import statements across the codebase.
Let's verify the impact:
✅ Verification successful
Let me gather more specific information about the imports and module usage.
Based on the search results, I can now generate the final response:
Removing "type": "module" is safe in this context
The codebase exclusively uses ES Module imports with TypeScript, which are handled by the TypeScript compiler configuration rather than Node.js's module system. All imports of @rallly/utils are using the ES Module import syntax, and there are no require() statements found. The removal of "type": "module" won't impact the existing imports because:
- All imports are using modern ES Module syntax (e.g.,
import { absoluteUrl } from "@rallly/utils/absolute-url") - The package is used within a TypeScript ecosystem where module resolution is controlled by
tsconfig.json - No CommonJS
require()statements were found in the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for ES module imports that might be affected
rg -g '*.{ts,js}' -l 'import .* from.*@rallly/utils'
# Check for any import/export statements in the source files
rg -g '*.{ts,js}' -l '^(import|export)' packages/utils/src/
Length of output: 613
Script:
#!/bin/bash
# Check how @rallly/utils is imported in the codebase
rg -g '*.{ts,js}' -A 2 'from.*@rallly/utils'
# Check the type of imports/exports in utils package
rg -g '*.{ts,js}' -A 2 '^(import|export)' packages/utils/src/
# Check if there are any require() statements
rg -g '*.{ts,js}' 'require\(["\']@rallly/utils'
# Check package.json files that depend on @rallly/utils
rg -g 'package.json' '"@rallly/utils":'
Length of output: 4202
apps/web/src/app/global-error.tsx (2)
7-11: LGTM! Clean formatting improvement.
The parameter destructuring has been formatted across multiple lines, improving readability while maintaining the same functionality.
Line range hint 1-27: LGTM! File maintains proper error handling.
The core error handling functionality with Sentry integration and NextError component remains correctly implemented, with only formatting improvements applied.
packages/emails/package.json (1)
9-9: Consider enhancing the Prettier integration.
The addition of Prettier is good, but consider these improvements:
- Add a check-only script for CI pipelines
- Add a combined script for all checks
- Ensure proper configuration is in place
Consider adding these scripts:
"scripts": {
"dev": "email dev --port 3333 --dir ./src/previews",
"lint": "eslint ./src",
"type-check": "tsc --pretty --noEmit",
"prettier": "prettier --write .",
+ "prettier:check": "prettier --check .",
+ "format": "prettier --write .",
+ "check": "npm run lint && npm run type-check && npm run prettier:check",
"i18n:scan": "i18next-scanner --config i18next-scanner.config.js"
}apps/web/declarations/next-auth.d.ts (1)
Line range hint 13-20: LGTM! Type definitions are well-structured.
The interface extensions are properly typed with optional properties and null unions, maintaining type safety across session, user, and JWT objects.
Also applies to: 22-28, 30-36
packages/ui/package.json (1)
11-11: LGTM! Good addition of the prettier script.
The addition of the prettier script aligns well with the PR's objective to enhance code formatting capabilities.
apps/web/src/app/api/storage/[...key]/route.ts (1)
2-2: LGTM! Import formatting aligns with PR objectives.
The type import formatting change is consistent with the PR's goal of improving code formatting and follows TypeScript best practices.
packages/emails/src/templates/new-comment.tsx (3)
3-3: LGTM! Import statement formatting looks good.
The consolidated import statement follows consistent formatting practices.
Line range hint 8-10: LGTM! Interface definition is well-structured.
The interface properly extends NotificationBaseProps and includes the necessary authorName property with appropriate typing.
Line range hint 47-58: LGTM! Static method implementation is well-structured.
The getSubject method is properly typed and correctly implements internationalization with variable interpolation.
apps/web/src/app/[locale]/poll/[urlId]/edit-settings/page.tsx (1)
9-10: LGTM! Import statements are properly formatted.
The changes align with the PR's objective of improving code formatting. The explicit type import for PollSettingsFormData follows TypeScript best practices by separating type imports from value imports.
packages/ui/src/avatar.tsx (1)
84-84: LGTM! Export statement formatting improved.
The spacing after commas in the export statement enhances readability and follows standard formatting conventions.
apps/web/src/components/add-to-calendar-button.tsx (1)
12-13: LGTM! Import statements properly organized.
The changes improve code organization by:
- Explicitly importing types using the
typekeyword - Consolidating related calendar function imports
apps/web/src/components/poll-context.tsx (1)
11-11: LGTM! Import changes are well-organized and properly utilized.
The added imports are consistent with the codebase usage:
ParsedTimeSlotOptiontype is used in type declarationsgetDurationfunction is used in thecreateOptionsContextValuefunction
Also applies to: 13-13
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
packages/eslint-config/preset.js (2)
21-21: Consider using template literals for consistency.While string concatenation works, template literals are more commonly used in modern JavaScript/TypeScript codebases.
- project: workspaceDirPath + "/tsconfig.json", + project: `${workspaceDirPath}/tsconfig.json`,
31-38: LGTM! Well-structured type import rules.The TypeScript type import configuration is well thought out. The rules will:
- Enforce clear separation between type and value imports
- Improve tree-shaking potential
- Maintain consistent import style across the codebase
Moving these rules to the TypeScript override section is the correct approach as they are TypeScript-specific.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
packages/eslint-config/.eslintrc.js(1 hunks)packages/eslint-config/preset.js(2 hunks)packages/ui/src/toast.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/eslint-config/.eslintrc.js
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/ui/src/toast.tsx
Summary by CodeRabbit
New Features
Mention,MentionedBy, andBigTestimonial.Bug Fixes
Chores
Documentation
package.jsonscripts for better clarity and organization.