fix: match edge runtime pages with optional trailing slash (#1892)

ascorbic · web-flow · commit 138b19d3bcaf · 2023-01-19T13:59:10.000Z
* fix: match edge runtime pages with optional trailing slash

* fix: handle trailing slash redirects
diff --git a/packages/runtime/src/helpers/edge.ts b/packages/runtime/src/helpers/edge.ts
@@ -202,27 +202,18 @@ const writeEdgeFunction = async ({
   edgeFunctionDefinition,
   edgeFunctionRoot,
   netlifyConfig,
-  pageRegexMap,
-  appPathRoutesManifest = {},
-  nextConfig,
-  cache,
+  functionName,
+  matchers = [],
+  middleware = false,
 }: {
   edgeFunctionDefinition: EdgeFunctionDefinition
   edgeFunctionRoot: string
   netlifyConfig: NetlifyConfig
-  pageRegexMap?: Map<string, string>
-  appPathRoutesManifest?: Record<string, string>
-  nextConfig: NextConfig
-  cache?: 'manual'
-}): Promise<
-  Array<{
-    function: string
-    name: string
-    pattern: string
-  }>
-> => {
-  const name = sanitizeName(edgeFunctionDefinition.name)
-  const edgeFunctionDir = join(edgeFunctionRoot, name)
+  functionName: string
+  matchers?: Array<MiddlewareMatcher>
+  middleware?: boolean
+}) => {
+  const edgeFunctionDir = join(edgeFunctionRoot, functionName)
 
   const bundle = await getMiddlewareBundle({
     edgeFunctionDefinition,
@@ -234,43 +225,52 @@ const writeEdgeFunction = async ({
 
   await copyEdgeSourceFile({
     edgeFunctionDir,
-    file: 'runtime.ts',
+    file: middleware ? 'middleware-runtime.ts' : 'function-runtime.ts',
     target: 'index.ts',
   })
 
-  const matchers: EdgeFunctionDefinitionV2['matchers'] = []
+  if (middleware) {
+    // Functions don't have complex matchers, so we can rely on the Netlify matcher
+    await writeJson(join(edgeFunctionDir, 'matchers.json'), matchers)
+  }
+}
 
+const generateEdgeFunctionMiddlewareMatchers = ({
+  edgeFunctionDefinition,
+  nextConfig,
+}: {
+  edgeFunctionDefinition: EdgeFunctionDefinition
+  edgeFunctionRoot: string
+  nextConfig: NextConfig
+  cache?: 'manual'
+}): Array<MiddlewareMatcher> => {
   // The v1 middleware manifest has a single regexp, but the v2 has an array of matchers
   if ('regexp' in edgeFunctionDefinition) {
-    matchers.push({ regexp: edgeFunctionDefinition.regexp })
-  } else if (nextConfig.i18n) {
-    matchers.push(
-      ...edgeFunctionDefinition.matchers.map((matcher) => ({
-        ...matcher,
-        regexp: makeLocaleOptional(matcher.regexp),
-      })),
-    )
-  } else {
-    matchers.push(...edgeFunctionDefinition.matchers)
+    return [{ regexp: edgeFunctionDefinition.regexp }]
   }
-
-  // If the EF matches a page, it's an app dir page so needs a matcher too
-  // The object will be empty if appDir isn't enabled in the Next config
-  if (pageRegexMap && edgeFunctionDefinition.page in appPathRoutesManifest) {
-    const regexp = pageRegexMap.get(appPathRoutesManifest[edgeFunctionDefinition.page])
-    if (regexp) {
-      matchers.push({ regexp })
-    }
+  if (nextConfig.i18n) {
+    return edgeFunctionDefinition.matchers.map((matcher) => ({
+      ...matcher,
+      regexp: makeLocaleOptional(matcher.regexp),
+    }))
   }
+  return edgeFunctionDefinition.matchers
+}
 
-  await writeJson(join(edgeFunctionDir, 'matchers.json'), matchers)
-
-  // We add a defintion for each matching path
-  return matchers.map((matcher) => {
-    const pattern = transformCaptureGroups(stripLookahead(matcher.regexp))
-    return { function: name, pattern, name: edgeFunctionDefinition.name, cache }
-  })
+const middlewareMatcherToEdgeFunctionDefinition = (
+  matcher: MiddlewareMatcher,
+  name: string,
+  cache?: 'manual',
+): {
+  function: string
+  name?: string
+  pattern: string
+  cache?: 'manual'
+} => {
+  const pattern = transformCaptureGroups(stripLookahead(matcher.regexp))
+  return { function: name, pattern, name, cache }
 }
+
 export const cleanupEdgeFunctions = ({
   INTERNAL_EDGE_FUNCTIONS_SRC = '.netlify/edge-functions',
 }: NetlifyPluginConstants) => emptyDir(INTERNAL_EDGE_FUNCTIONS_SRC)
@@ -348,9 +348,28 @@ export const writeRscDataEdgeFunction = async ({
   ]
 }
 
+const getEdgeFunctionPatternForPage = ({
+  edgeFunctionDefinition,
+  pageRegexMap,
+  appPathRoutesManifest,
+}: {
+  edgeFunctionDefinition: EdgeFunctionDefinitionV2
+  pageRegexMap: Map<string, string>
+  appPathRoutesManifest?: Record<string, string>
+}): string => {
+  // We don't just use the matcher from the edge function definition, because it doesn't handle trailing slashes
+
+  // appDir functions have a name that _isn't_ the route name, but rather the route with `/page` appended
+  const regexp = pageRegexMap.get(appPathRoutesManifest?.[edgeFunctionDefinition.page] ?? edgeFunctionDefinition.page)
+  // If we need to fall back to the matcher, we need to add an optional trailing slash
+  return regexp ?? edgeFunctionDefinition.matchers[0].regexp.replace(/([^/])\$$/, '$1/?$')
+}
+
 /**
  * Writes Edge Functions for the Next middleware
  */
+
+// eslint-disable-next-line max-lines-per-function
 export const writeEdgeFunctions = async ({
   netlifyConfig,
   routesManifest,
@@ -415,16 +434,25 @@ export const writeEdgeFunctions = async ({
     for (const middleware of middlewareManifest.sortedMiddleware) {
       usesEdge = true
       const edgeFunctionDefinition = middlewareManifest.middleware[middleware]
-      const functionDefinitions = await writeEdgeFunction({
+      const functionName = sanitizeName(edgeFunctionDefinition.name)
+      const matchers = generateEdgeFunctionMiddlewareMatchers({
         edgeFunctionDefinition,
         edgeFunctionRoot,
-        netlifyConfig,
         nextConfig,
       })
-      manifest.functions.push(...functionDefinitions)
+      await writeEdgeFunction({
+        edgeFunctionDefinition,
+        edgeFunctionRoot,
+        netlifyConfig,
+        functionName,
+        matchers,
+        middleware: true,
+      })
+
+      manifest.functions.push(
+        ...matchers.map((matcher) => middlewareMatcherToEdgeFunctionDefinition(matcher, functionName)),
+      )
     }
-    // Older versions of the manifest format don't have the functions field
-    // No, the version field was not incremented
     if (typeof middlewareManifest.functions === 'object') {
       // When using the app dir, we also need to check if the EF matches a page
       const appPathRoutesManifest = await loadAppPathRoutesManifest(netlifyConfig)
@@ -438,17 +466,25 @@ export const writeEdgeFunctions = async ({
 
       for (const edgeFunctionDefinition of Object.values(middlewareManifest.functions)) {
         usesEdge = true
-        const functionDefinitions = await writeEdgeFunction({
+        const functionName = sanitizeName(edgeFunctionDefinition.name)
+        await writeEdgeFunction({
           edgeFunctionDefinition,
           edgeFunctionRoot,
           netlifyConfig,
+          functionName,
+        })
+        const pattern = getEdgeFunctionPatternForPage({
+          edgeFunctionDefinition,
           pageRegexMap,
           appPathRoutesManifest,
-          nextConfig,
+        })
+        manifest.functions.push({
+          function: functionName,
+          name: edgeFunctionDefinition.name,
+          pattern,
           // cache: "manual" is currently experimental, so we restrict it to sites that use experimental appDir
           cache: usesAppDir ? 'manual' : undefined,
         })
-        manifest.functions.push(...functionDefinitions)
       }
     }
     if (usesEdge) {
diff --git a/packages/runtime/src/templates/edge-shared/utils.test.ts b/packages/runtime/src/templates/edge-shared/utils.test.ts
@@ -1,6 +1,6 @@
 import { assertEquals } from 'https://deno.land/std@0.167.0/testing/asserts.ts'
 import { beforeEach, describe, it } from 'https://deno.land/std@0.167.0/testing/bdd.ts'
-import { updateModifiedHeaders, FetchEventResult } from './utils.ts'
+import { redirectTrailingSlash, updateModifiedHeaders } from './utils.ts'
 
 describe('updateModifiedHeaders', () => {
   it("does not modify the headers if 'x-middleware-override-headers' is not found", () => {
@@ -62,3 +62,53 @@ describe('updateModifiedHeaders', () => {
     })
   })
 })
+
+describe('trailing slash redirects', () => {
+  it('adds a trailing slash to the pathn if trailingSlash is enabled', () => {
+    const url = new URL('https://example.com/foo')
+    const result = redirectTrailingSlash(url, true)
+    assertEquals(result?.status, 308)
+    assertEquals(result?.headers.get('location'), 'https://example.com/foo/')
+  })
+
+  it("doesn't add a trailing slash if trailingSlash is false", () => {
+    const url = new URL('https://example.com/foo')
+    const result = redirectTrailingSlash(url, false)
+    assertEquals(result, undefined)
+  })
+
+  it("doesn't add a trailing slash if the path is a file", () => {
+    const url = new URL('https://example.com/foo.txt')
+    const result = redirectTrailingSlash(url, true)
+    assertEquals(result, undefined)
+  })
+  it('adds a trailing slash if there is a dot in the path', () => {
+    const url = new URL('https://example.com/foo.bar/baz')
+    const result = redirectTrailingSlash(url, true)
+    assertEquals(result?.status, 308)
+    assertEquals(result?.headers.get('location'), 'https://example.com/foo.bar/baz/')
+  })
+  it("doesn't add a trailing slash if the path is /", () => {
+    const url = new URL('https://example.com/')
+    const result = redirectTrailingSlash(url, true)
+    assertEquals(result, undefined)
+  })
+  it('removes a trailing slash from the path if trailingSlash is false', () => {
+    const url = new URL('https://example.com/foo/')
+    const result = redirectTrailingSlash(url, false)
+    assertEquals(result?.status, 308)
+    assertEquals(result?.headers.get('location'), 'https://example.com/foo')
+  })
+  it("doesn't remove a trailing slash if trailingSlash is true", () => {
+    const url = new URL('https://example.com/foo/')
+    const result = redirectTrailingSlash(url, true)
+    assertEquals(result, undefined)
+  })
+
+  it('removes a trailing slash from the path if the path is a file', () => {
+    const url = new URL('https://example.com/foo.txt/')
+    const result = redirectTrailingSlash(url, false)
+    assertEquals(result?.status, 308)
+    assertEquals(result?.headers.get('location'), 'https://example.com/foo.txt')
+  })
+})
diff --git a/packages/runtime/src/templates/edge-shared/utils.ts b/packages/runtime/src/templates/edge-shared/utils.ts
@@ -56,6 +56,37 @@ interface MiddlewareRequest {
   rewrite(destination: string | URL, init?: ResponseInit): Response
 }
 
+export interface I18NConfig {
+  defaultLocale: string
+  localeDetection?: false
+  locales: string[]
+}
+
+export interface RequestData {
+  geo?: {
+    city?: string
+    country?: string
+    region?: string
+    latitude?: string
+    longitude?: string
+    timezone?: string
+  }
+  headers: Record<string, string>
+  ip?: string
+  method: string
+  nextConfig?: {
+    basePath?: string
+    i18n?: I18NConfig | null
+    trailingSlash?: boolean
+  }
+  page?: {
+    name?: string
+    params?: { [key: string]: string }
+  }
+  url: string
+  body?: ReadableStream<Uint8Array>
+}
+
 function isMiddlewareRequest(response: Response | MiddlewareRequest): response is MiddlewareRequest {
   return 'originalRequest' in response
 }
@@ -90,6 +121,34 @@ export function updateModifiedHeaders(requestHeaders: Headers, responseHeaders:
   responseHeaders.delete('x-middleware-override-headers')
 }
 
+export const buildNextRequest = (
+  request: Request,
+  context: Context,
+  nextConfig?: RequestData['nextConfig'],
+): RequestData => {
+  const { url, method, body, headers } = request
+
+  const { country, subdivision, city, latitude, longitude, timezone } = context.geo
+
+  const geo: RequestData['geo'] = {
+    country: country?.code,
+    region: subdivision?.code,
+    city,
+    latitude: latitude?.toString(),
+    longitude: longitude?.toString(),
+    timezone,
+  }
+
+  return {
+    headers: Object.fromEntries(headers.entries()),
+    geo,
+    url,
+    method,
+    ip: context.ip,
+    body: body ?? undefined,
+    nextConfig,
+  }
+}
 export const buildResponse = async ({
   result,
   request,
@@ -196,3 +255,19 @@ export const buildResponse = async ({
   }
   return res
 }
+
+export const redirectTrailingSlash = (url: URL, trailingSlash: boolean): Response | undefined => {
+  const { pathname } = url
+  if (pathname === '/') {
+    return
+  }
+  if (!trailingSlash && pathname.endsWith('/')) {
+    url.pathname = pathname.slice(0, -1)
+    return Response.redirect(url, 308)
+  }
+  // Add a slash, unless there's a file extension
+  if (trailingSlash && !pathname.endsWith('/') && !pathname.split('/').pop()?.includes('.')) {
+    url.pathname = `${pathname}/`
+    return Response.redirect(url, 308)
+  }
+}
diff --git a/packages/runtime/src/templates/edge/function-runtime.ts b/packages/runtime/src/templates/edge/function-runtime.ts
@@ -0,0 +1,23 @@
+import type { Context } from 'https://edge.netlify.com'
+// Available at build time
+import edgeFunction from './bundle.js'
+import { buildNextRequest, buildResponse, redirectTrailingSlash } from '../edge-shared/utils.ts'
+import nextConfig from '../edge-shared/nextConfig.json' assert { type: 'json' }
+
+const handler = async (req: Request, context: Context) => {
+  const url = new URL(req.url)
+  const redirect = redirectTrailingSlash(url, nextConfig.trailingSlash)
+  if (redirect) {
+    return redirect
+  }
+  const request = buildNextRequest(req, context, nextConfig)
+  try {
+    const result = await edgeFunction({ request })
+    return buildResponse({ result, request: req, context })
+  } catch (error) {
+    console.error(error)
+    return new Response(error.message, { status: 500 })
+  }
+}
+
+export default handler
diff --git a/packages/runtime/src/templates/edge/middleware-runtime.ts b/packages/runtime/src/templates/edge/middleware-runtime.ts
diff --git a/test/e2e/modified-tests/streaming-ssr/index.test.ts b/test/e2e/modified-tests/streaming-ssr/index.test.ts
