draft #120
draft #120
Conversation
❌ Deploy Preview for dtp-stat failed. Why did it fail? →
|
WalkthroughThe export of the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/shared/django-helpers.tsxOops! Something went wrong! :( ESLint: 8.57.1 TypeError: Cannot read properties of undefined (reading 'getAllComments') Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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}
| // export const djangoBaseUrl = process.env.NEXT_PUBLIC_DJANGO_BASE_URL ?? ""; | ||
| export const djangoBaseUrl = window.location.origin ?? ""; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Critical: SSR compatibility issue and security concern.
This change introduces two critical problems:
-
SSR Error:
window.location.originwill cause "ReferenceError: window is not defined" during server-side rendering in Next.js. -
Security Risk: Using the current window origin as the Django base URL means the application will trust whatever domain it's running on, potentially enabling malicious redirects.
Consider reverting to the environment variable approach or add proper runtime guards:
-// export const djangoBaseUrl = process.env.NEXT_PUBLIC_DJANGO_BASE_URL ?? "";
-export const djangoBaseUrl = window.location.origin ?? "";
+export const djangoBaseUrl = process.env.NEXT_PUBLIC_DJANGO_BASE_URL ?? "";If runtime determination is required, use a function with proper guards:
-export const djangoBaseUrl = window.location.origin ?? "";
+export const getDjangoBaseUrl = (): string => {
+ if (typeof window === 'undefined') {
+ return process.env.NEXT_PUBLIC_DJANGO_BASE_URL ?? "";
+ }
+ return process.env.NEXT_PUBLIC_DJANGO_BASE_URL ?? window.location.origin ?? "";
+};Let me verify how this constant is used across the codebase:
🏁 Script executed:
#!/bin/bash
# Description: Check usage of djangoBaseUrl to assess impact of the change
# Expected: Find all usages and determine if they're in SSR contexts
echo "=== Searching for djangoBaseUrl usage ==="
rg -A 3 -B 3 "djangoBaseUrl" --type tsx --type ts --type js --type jsx
echo -e "\n=== Checking for SSR-related files that might import this ==="
rg -l "django-helpers" --type tsx --type ts --type js --type jsx | head -10
echo -e "\n=== Checking if this file is imported in _app or layout files ==="
rg -A 2 -B 2 "django-helpers" pages/_app.* app/layout.* || echo "No _app or layout imports found"Length of output: 820
#!/bin/bash
Search for all import sites of the shared django-helpers module
echo "=== Imports of src/shared/django-helpers.tsx ==="
rg "from .*shared/django-helpers" -n
List any pages or API routes using SSR data-fetching methods
echo -e "\n=== Files defining getServerSideProps ==="
rg "getServerSideProps" -n
echo -e "\n=== Files defining getStaticProps ==="
rg "getStaticProps" -n
Check if any SSR file imports django-helpers
echo -e "\n=== SSR files that import shared/django-helpers.tsx ==="
rg "from .*shared/django-helpers" -l | xargs -r rg -n "getServerSideProps|getStaticProps"
#!/bin/bash
Search for all usages of djangoBaseUrl in the repository
echo "=== Searching for djangoBaseUrl usages ==="
rg "djangoBaseUrl" -n .
Search for imports of the django-helpers module
echo -e "\n=== Searching for imports of shared/django-helpers.tsx ==="
rg "shared/django-helpers" -n .
List all files using Next.js SSR/SSG methods
echo -e "\n=== Files with getServerSideProps ==="
rg "getServerSideProps" -n .
echo -e "\n=== Files with getStaticProps ==="
rg "getStaticProps" -n .
echo -e "\n=== Files with getInitialProps ==="
rg "getInitialProps" -n .
🏁 Script executed:
#!/bin/bash
# Search for all references to djangoBaseUrl
echo "=== djangoBaseUrl usages ==="
rg "djangoBaseUrl" -n -g "*.{ts,tsx,js,jsx}"
# Search for all import sites of the shared helper
echo -e "\n=== Imports of shared/django-helpers.tsx ==="
rg "shared/django-helpers" -n -g "*.{ts,tsx,js,jsx}"
# Identify any SSR/SSG files that import this helper
echo -e "\n=== SSR/SSG files importing django-helpers ==="
for f in $(rg -l "shared/django-helpers" -g "*.{ts,tsx,js,jsx}"); do
if rg -q "getServerSideProps|getStaticProps|getInitialProps" "$f"; then
echo "$f"
fi
doneLength of output: 3713
Fix SSR/Server-side Import of djangoBaseUrl
The switch to window.location.origin breaks any server‐side or API‐route imports of djangoBaseUrl (e.g. src/pages/api/rewrites/proxy-django-html-page.tsx), since window is undefined in Node and during SSR.
• Affected locations:
- src/shared/django-helpers.tsx
- src/pages/api/rewrites/proxy-django-html-page.tsx
- (also client components sharing the same constant)
• Proposed fix: turn the constant into a guarded function that falls back to the env var on the server:
// src/shared/django-helpers.tsx
export const getDjangoBaseUrl = (): string => {
if (typeof window !== "undefined") {
return window.location.origin;
}
return process.env.NEXT_PUBLIC_DJANGO_BASE_URL ?? "";
};Then update all imports/usages:
- import { djangoBaseUrl } from "../shared/django-helpers";
+ import { getDjangoBaseUrl } from "../shared/django-helpers";- void router.replace(`${djangoBaseUrl}${djangoPageHref}`);
+ void router.replace(`${getDjangoBaseUrl()}${djangoPageHref}`);And in the API route:
- import { djangoBaseUrl } from "../../../shared/django-helpers";
+ import { getDjangoBaseUrl } from "../../../shared/django-helpers";
- const upstreamUrl = new URL(req.url ?? "/", djangoBaseUrl);
+ const upstreamUrl = new URL(req.url ?? "/", getDjangoBaseUrl());Optionally, use a non-public server‐only env var (e.g. DJANGO_BASE_URL) instead of a NEXT_PUBLIC_ one for stronger configuration control.
🤖 Prompt for AI Agents
In src/shared/django-helpers.tsx around lines 15 to 16, the use of
window.location.origin breaks server-side rendering and API route imports
because window is undefined in Node. Replace the exported constant djangoBaseUrl
with a function getDjangoBaseUrl that returns window.location.origin if window
is defined, otherwise falls back to process.env.NEXT_PUBLIC_DJANGO_BASE_URL or
an empty string. Update all imports and usages of djangoBaseUrl to call
getDjangoBaseUrl() instead. Optionally, consider using a non-public server-only
environment variable for stronger configuration control.
There was a problem hiding this comment.
@alexander-mart Для получения текущего домена в SSR и поддержки нескольких доменов, вам нужно использовать заголовки запроса. Вот несколько подходов:
1. Универсальная функция для получения домена
// src/shared/django-helpers.tsx
export const getDjangoBaseUrl = (req?: any): string => {
// В SSR контексте используем заголовки запроса
if (typeof window === 'undefined' && req) {
const protocol = req.headers['x-forwarded-proto'] || 'http';
const host = req.headers.host;
return `${protocol}://${host}`;
}
// В браузере используем window.location
if (typeof window !== 'undefined') {
return window.location.origin;
}
// Fallback для случаев без контекста
return process.env.NEXT_PUBLIC_DJANGO_BASE_URL ?? '';
};2. В getServerSideProps
export async function getServerSideProps({ req }: GetServerSidePropsContext) {
const baseUrl = getDjangoBaseUrl(req);
// Теперь API запросы идут к тому же домену
const response = await fetch(`${baseUrl}/api/stat`);
const data = await response.json();
return {
props: {
data,
baseUrl
}
};
}3. В API routes
// pages/api/proxy-stats.ts
import type { NextApiRequest, NextApiResponse } from 'next';
export default async function handler(req: NextApiRequest, res: NextApiResponse) {
const protocol = req.headers['x-forwarded-proto'] || 'http';
const host = req.headers.host;
const baseUrl = `${protocol}://${host}`;
// Проксируем запрос к Django на том же домене
const response = await fetch(`${baseUrl}/api/stat`);
const data = await response.json();
res.json(data);
}4. Для клиентских запросов без CORS
// src/hooks/useApi.ts
import { getDjangoBaseUrl } from '../shared/django-helpers';
export const useApi = () => {
const makeRequest = async (endpoint: string) => {
const baseUrl = getDjangoBaseUrl();
// Запрос идёт к тому же домену - нет CORS проблем
const response = await fetch(`${baseUrl}${endpoint}`);
return response.json();
};
return { makeRequest };
};5. С использованием Next.js middleware (рекомендуется)
// middleware.ts
import { NextRequest, NextResponse } from 'next/server';
export function middleware(request: NextRequest) {
const response = NextResponse.next();
// Добавляем текущий домен в заголовки для использования в компонентах
response.headers.set('x-current-domain', request.nextUrl.origin);
return response;
}
export const config = {
matcher: ['/((?!api|_next/static|_next/image|favicon.ico).*)'],
};Затем в компонентах:
// src/components/MyComponent.tsx
import { headers } from 'next/headers';
export default function MyComponent() {
const headersList = headers();
const currentDomain = headersList.get('x-current-domain') || '';
// Используем текущий домен для API запросов
const apiUrl = `${currentDomain}/api/stat`;
return <div>{/* ваш компонент */}</div>;
}Обновлённая версия django-helpers.tsx
// src/shared/django-helpers.tsx
export const getDjangoBaseUrl = (req?: any): string => {
// SSR с объектом запроса
if (typeof window === 'undefined' && req) {
const protocol = req.headers['x-forwarded-proto'] ||
req.connection?.encrypted ? 'https' : 'http';
const host = req.headers.host;
return `${protocol}://${host}`;
}
// Браузер
if (typeof window !== 'undefined') {
return window.location.origin;
}
// Fallback
return process.env.NEXT_PUBLIC_DJANGO_BASE_URL ?? '';
};Такой подход:
- ✅ Работает в SSR и клиенте
- ✅ Поддерживает множественные домены автоматически
- ✅ Избегает CORS ошибок (запросы к тому же домену)
- ✅ Не ломает сборку Next.js
✏️ Learnings added
Learnt from: alexander-mart
PR: dtpstat/website#120
File: src/shared/django-helpers.tsx:15-16
Timestamp: 2025-07-24T20:42:58.703Z
Learning: В Next.js для получения текущего домена в SSR используется req.headers.host из объекта запроса. Для избежания CORS ошибок при работе с несколькими доменами, API запросы должны идти к тому же домену, что и текущая страница.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
Summary by CodeRabbit