-
Notifications
You must be signed in to change notification settings - Fork 0
draft #121
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
draft #121
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes ✨ 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 (
|
❌ Deploy Preview for dtp-stat failed. Why did it fail? →
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/shared/django-helpers.tsx (1)
114-124: Architectural consideration: Client-side usage requires a different approach.The
djangoBaseUrlis used in client-side React components (e.g.,DjangoRedirectOnIframeAuth). SincegetUrlrequires server-side request context, you'll need to:
- Pass the base URL from server-side props to client components
- Use a client-side method to determine the current URL (e.g.,
window.location.origin)- Or maintain the environment variable approach for client-side usage
Consider whether dynamic URL resolution is truly needed for your use case, given the client-side constraints.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/shared/django-helpers.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/shared/django-helpers.tsx (1)
Learnt from: alexander-mart
PR: #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 запросы должны идти к тому же домену, что и текущая страница.
| import Script from "next/script"; | ||
| import * as React from "react"; | ||
|
|
||
| import { getUrl } from 'nextjs-current-url'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify if nextjs-current-url is the appropriate package for this use case.
Based on the retrieved learnings, Next.js SSR should use req.headers.host to get the current domain. Please verify that this package properly handles SSR contexts and provides the correct domain for avoiding CORS issues.
🌐 Web query:
What is the nextjs-current-url npm package and how does it work for getting URLs in Next.js SSR context?
💡 Result:
The nextjs-current-url npm package enables you to retrieve the current URL or route in Next.js applications, in both client-side and server-side environments—including in SSR (Server-Side Rendering) contexts[9].
How nextjs-current-url Works
-
Purpose:
The package offers utility functions to quickly get the full URL, including protocol, host, path, and query string, regardless of whether your code runs on the server or client[9]. -
Client Side:
On the client, it typically useswindow.locationto access the full URL. -
Server Side (SSR):
On the server, it extracts URL information from the request object (req) available in server-rendering methods likegetServerSidePropsor API routes[2]. This way, you can determine the full URL as seen by the user at request time, which is very useful for SSR scenarios.
Example Usage (SSR Context)
Suppose you use getServerSideProps in a page:
import { getCurrentUrl } from "nextjs-current-url";
export async function getServerSideProps(context) {
const url = getCurrentUrl(context); // Retrieves full URL in SSR
return { props: { url } };
}- Here,
contextcontains the underlying Node.jsreqandresobjects. The package reads from these to construct the correct URL, handling edge cases like proxies or Vercel deployments automatically[9].
When to Use
- SSR Redirections
- Canonical URL calculation
- Generating absolute links or sharing URLs during server-rendered requests
Similar Alternatives
- Manual construction with
reqobject: You can build the URL manually fromreq.headers, but this requires boilerplate and does not handle all deployment edge cases automatically[2][1]. - Other packages: Packages like
nextjs-urlprovide a similar API for extracting protocol, host, and path from the request both on server and client[1].
Limitations and Notes
- The package must be used inside SSR/server methods (like
getServerSideProps,getInitialProps, or API routes) to access thereqobject. It will not work for static site generation (SSG), because at build time there is no incoming HTTP request[1]. - For Next.js 13+ and App Router with server components, you may need to check compatibility or use alternative methods, as access to the full request object may differ[5][6].
In summary:
nextjs-current-url is a convenient utility for getting the entire current URL in Next.js, abstracting away the differences between client and server environments in SSR[9]. It is especially useful in dynamic, server-rendered pages where deducing the user's actual request URL is crucial.
Citations:
- 1: https://github.com/tajultonim/nextjs-url
- 2: https://www.squash.io/how-to-retrieve-remote-urls-with-nextjs/
- 3: https://www.geeksforgeeks.org/reactjs/how-to-get-the-current-url-domain-in-next-js/
- 4: https://blog.appsignal.com/2023/12/13/server-side-rendering-with-nextjs-react-and-typescript.html
- 5: Is there a way to grab the current URL in a nextjs 13 Layout component vercel/next.js#56572
- 6: https://www.propelauth.com/post/getting-url-in-next-server-components
- 7: https://nextjs.org/docs/pages/api-reference/components/link
- 8: https://nextjs.org/docs/pages/building-your-application/routing/linking-and-navigating
- 9: https://www.npmjs.com/package/nextjs-current-url/v/1.0.1
- 10: https://nextjs.org/docs/app/getting-started/linking-and-navigating
🏁 Script executed:
#!/bin/bash
# Locate django-helpers.tsx and show its content
fd django-helpers.tsx -x sed -n '1,200p'Length of output: 5086
Fix SSR retrieval of the Django base URL
The nextjs-current-url package is perfectly capable of extracting the full URL in both client and SSR contexts, but it must be invoked inside a Next.js SSR lifecycle method (e.g. getServerSideProps) with the actual req object—not at module initialization with context.req.host, which doesn’t exist here. The current line will throw at build/runtime and won’t reliably prevent CORS issues.
• File: src/shared/django-helpers.tsx
• Line: definition of djangoBaseUrl
Suggested fix:
-// Module-scope (broken): no `context`, `req` is just a host string
-export const djangoBaseUrl = getUrl({ req: context.req.host }) ?? '';
+// pages/yourPage.tsx (or move to a custom SSR helper)
+export async function getServerSideProps(context) {
+ // Pass the full req object to getUrl
+ const djangoBaseUrl = getUrl({ req: context.req }) ?? '';
+ return { props: { djangoBaseUrl } };
+}
+
+// In your component, consume the prop instead of the top-level constant
+// export const djangoBaseUrl = props.djangoBaseUrl;Alternatively, construct the base URL manually in SSR:
export async function getServerSideProps({ req }) {
const protocol = req.headers['x-forwarded-proto'] || 'https';
const host = req.headers.host;
return { props: { djangoBaseUrl: `${protocol}://${host}` } };
}• Ensure all downstream code (e.g. DjangoRedirectOnIframeAuth) reads djangoBaseUrl from props or context where SSR has run.
• Keep the import of getUrl if you move it into SSR—otherwise remove unused imports.
• This change guarantees you’re using the real request host (per req.headers.host) to avoid CORS issues during SSR.
🤖 Prompt for AI Agents
In src/shared/django-helpers.tsx at line 13, the current approach to define
djangoBaseUrl uses nextjs-current-url at module initialization, which fails in
SSR because context.req.host is unavailable. To fix this, remove the
djangoBaseUrl definition from the module scope and instead obtain the base URL
inside a Next.js SSR lifecycle method like getServerSideProps by extracting
protocol and host from req.headers, then pass djangoBaseUrl as a prop or via
context to downstream components. Also, remove the getUrl import if it is no
longer used outside SSR.
| // export const djangoBaseUrl = process.env.NEXT_PUBLIC_DJANGO_BASE_URL ?? ""; | ||
| export const djangoBaseUrl = getUrl({ req: context.req.host }) ?? ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Critical: context is undefined and implementation is incorrect for module-level export.
The current implementation has several critical issues:
contextis not defined anywhere, which will cause aReferenceErrordjangoBaseUrlis a module-level export that cannot access request context- The syntax
getUrl({ req: context.req.host })appears incorrect
This change from a static environment variable to dynamic URL retrieval fundamentally changes how djangoBaseUrl can be used. You'll need to either:
- Keep it as an environment variable for module-level usage
- Refactor all usages to pass the request context and compute the URL dynamically
Let me verify how djangoBaseUrl is currently used in the codebase:
🏁 Script executed:
#!/bin/bash
# Description: Check all usages of djangoBaseUrl to understand the refactoring scope
# Search for all imports and usages of djangoBaseUrl
rg -A 3 "djangoBaseUrl" --type tsx --type ts
# Check if there are any Next.js API routes or getServerSideProps that could provide context
fd -e tsx -e ts -x grep -l "getServerSideProps\|getStaticProps\|export.*async.*function.*handler" {} \;Length of output: 181
🏁 Script executed:
#!/bin/bash
# Locate the django-helpers file
file_path=$(fd -t f -e tsx -e ts "django-helpers.tsx" . | head -n 1)
echo "Found file: $file_path"
# Show the top of the file to verify 'context' usage
if [[ -n "$file_path" ]]; then
sed -n '1,200p' "$file_path"
else
echo "django-helpers.tsx not found"
fi
# Search for all references to djangoBaseUrl in the repo
rg "djangoBaseUrl" -n .
# Search for Next.js data-fetching methods or API route handlers that might provide context
rg -n "getServerSideProps|getStaticProps|export\\s+default\\s+function\\s+handler" .Length of output: 6903
Critical: context is undefined and cannot be used at module level
The export on line 18 of src/shared/django-helpers.tsx
export const djangoBaseUrl = getUrl({ req: context.req.host }) ?? '';will throw a ReferenceError because context is never declared—and at module load time there is no Next.js request context available. All of the 10+ import sites (e.g. src/pages/api/rewrites/proxy-django-html-page.tsx, src/components/inherited-map/api.ts, etc.) will break.
You must either:
- Revert to using the environment variable for module-level code:
- export const djangoBaseUrl = getUrl({ req: context.req.host }) ?? ''; + export const djangoBaseUrl = process.env.NEXT_PUBLIC_DJANGO_BASE_URL ?? '';
- Or refactor
djangoBaseUrlinto a function that accepts the Next.js request (or context) at runtime, for example:and update all call sites to invokeexport function getDjangoBaseUrl(req: NextApiRequest | { host: string }): string { return getUrl({ req }) ?? process.env.NEXT_PUBLIC_DJANGO_BASE_URL ?? ''; }
getDjangoBaseUrl(req)instead of importing a top-level constant.
🤖 Prompt for AI Agents
In src/shared/django-helpers.tsx at lines 17-18, the export of djangoBaseUrl
uses an undefined variable context at the module level, causing a
ReferenceError. To fix this, remove the module-level usage of context and either
revert djangoBaseUrl to use the environment variable directly or refactor it
into a function that accepts the request or context at runtime. Update all
import sites to call this new function with the appropriate request object
instead of importing a constant.
Summary by CodeRabbit