NEX-179: Move the redirect logic to middleware #290
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Work in progress: * console.log messages left on purpose * one call to drupal for each page + prefetch links -> not great
vermario
commented
Apr 29, 2025
There was a problem hiding this comment.
Basically here the idea is that once we get here we dont' need to check if there's a redirect, because that is handled by the middleware.
vermario
commented
Apr 29, 2025
Comment on lines
-14
to
-35
| /** | ||
| * Function to directly fetch a node from Drupal by its path and locale. | ||
| * | ||
| * @param path The path of the node. | ||
| * @param locale The locale of the node. | ||
| * @param revision The revision of the node. | ||
| * @param isDraftMode If true, fetches the draft version of the node. | ||
| * @returns The fetched node data or null if not found. | ||
| */ | ||
| export async function fetchNodeByPathQuery( | ||
| path: string, | ||
| locale: string, | ||
| isDraftMode: boolean, | ||
| revision: string = null, | ||
| ) { | ||
| const drupalClient = isDraftMode ? drupalClientPreviewer : drupalClientViewer; | ||
| return await drupalClient.doGraphQlRequest(GET_ENTITY_AT_DRUPAL_PATH, { | ||
| path, | ||
| langcode: locale, | ||
| revision, | ||
| }); | ||
| } |
There was a problem hiding this comment.
I had to move this function to its own file, because we could not import an export from this file in middleware (the neshka/cache package causes webpack errors there)
vermario
commented
Apr 29, 2025
There was a problem hiding this comment.
I had to move this function to its own file, because we could not import an export from this file in middleware (the neshka/cache package causes webpack errors there)
vermario
commented
Apr 29, 2025
Comment on lines
+31
to
+103
| // Handle redirects from Drupal | ||
| async function handleDrupalRedirects(request: NextRequest) { | ||
| const pathname = request.nextUrl.pathname; | ||
| console.log("Handling Drupal redirects for path:", pathname); | ||
|
|
||
| // We need to check if the path includes a locale. | ||
| // Get the default locale from the routing configuration | ||
| const defaultLocale = routing.defaultLocale; | ||
| const pathnameSegments = pathname.split("/").filter(Boolean); | ||
|
|
||
| // Handle locale determination | ||
| let locale; | ||
| let slugSegments; | ||
|
|
||
| if (pathnameSegments.length === 0) { | ||
| // Root path - use default locale | ||
| locale = defaultLocale; | ||
| slugSegments = []; | ||
| } else { | ||
| // Check if first segment is a valid locale | ||
| const firstSegment = pathnameSegments[0]; | ||
| const isLocale = routing.locales.includes( | ||
| firstSegment as (typeof routing.locales)[number], | ||
| ); | ||
|
|
||
| if (isLocale) { | ||
| // Path includes locale: /en/about | ||
| locale = firstSegment; | ||
| slugSegments = pathnameSegments.slice(1); | ||
| } else { | ||
| // Path doesn't include locale: /about | ||
| // Assume default locale | ||
| locale = defaultLocale; | ||
| slugSegments = pathnameSegments; | ||
| } | ||
| } | ||
|
|
||
| // If there are no slug segments, it's likely the homepage | ||
| if (slugSegments.length === 0) { | ||
| return null; | ||
| } | ||
|
|
||
| // Construct path for query | ||
| const path = "/" + slugSegments.join("/"); | ||
| console.log("Constructed path for query:", path); | ||
| console.log("Locale for query:", locale); | ||
|
|
||
| try { | ||
| // Check if this path should redirect | ||
| const nodeByPathResult = await fetchNodeByPathQuery(path, locale, false); | ||
| const redirectResult = | ||
| extractRedirectFromRouteQueryResult(nodeByPathResult); | ||
|
|
||
| if (redirectResult) { | ||
| // Apply the appropriate redirect | ||
| const status = | ||
| redirectResult.status === 307 || redirectResult.status === 302 | ||
| ? redirectResult.status | ||
| : 308; // Permanent redirect | ||
|
|
||
| return NextResponse.redirect( | ||
| new URL(redirectResult.url, request.url), | ||
| status, | ||
| ); | ||
| } | ||
| } catch (error) { | ||
| // If there's an error, continue to the page component | ||
| console.error("Middleware error:", error); | ||
| } | ||
|
|
||
| return null; | ||
| } | ||
|
|
There was a problem hiding this comment.
Here we handle the logic to understand if the path has the language in it en/my-drupal-page, and if it does not we assume the default locale (so /drupal-page is the same as en/drupal-page
also we ignore the frontpage
vermario
commented
Apr 29, 2025
Comment on lines
+139
to
+141
| // Check if there are redirects from Drupal for this request: | ||
| const redirectResponse = await handleDrupalRedirects(request); | ||
| if (redirectResponse) { |
There was a problem hiding this comment.
I added this here, after the checks for auth pages, and before the middleware for next-intl
(not sure this is great)
vermario
commented
Apr 29, 2025
Comment on lines
-80
to
-84
| // If the node is a frontpage, redirect to the frontpage: | ||
| if (!isDraftMode && node.__typename === "NodeFrontpage") { | ||
| redirect(`/${locale}`); | ||
| } | ||
|
|
There was a problem hiding this comment.
ah, also this we would need to handle in middleware maybe
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
WORK IN PROGRESS
The issue
In page router, we could check for the existence of a redirect when rendering a page with getStaticProps: the redirect response would them be cached an we could live happily.
In the app router, we cannot return a redirect from a static page in any case. (This was hard to spot, since when running in dev mode it works happily)
So the solutions as far as I understand are:
Why it's not great
The solution here seems to work:
This works I think, but if the idea is to protect the backend from too many queries, this means again that for one page view there is at least one more query to Drupal. Also, when there are links on the page and those are prefetched, those are also all checked (I left the console.log messages so they can be seen).
I wonder if this is a necessary evil, or if there are ways to improve this.
For example can we cache on the frontend these redirect calls? but using which logic and how do we invalidate them? (maybe in-memory cache and using our usual REVALIDATE_LONG env var?)