Skip to content

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

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

Merged
merged 3 commits into from
Jan 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
140 changes: 88 additions & 52 deletions packages/runtime/src/helpers/edge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand All @@ -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) {
Expand Down
52 changes: 51 additions & 1 deletion packages/runtime/src/templates/edge-shared/utils.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { assertEquals } from 'https://deno.land/[email protected]/testing/asserts.ts'
import { beforeEach, describe, it } from 'https://deno.land/[email protected]/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", () => {
Expand Down Expand Up @@ -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')
})
})
75 changes: 75 additions & 0 deletions packages/runtime/src/templates/edge-shared/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
}
}
23 changes: 23 additions & 0 deletions packages/runtime/src/templates/edge/function-runtime.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import type { Context } from 'https://edge.netlify.com'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've split this into a separate entrypoint, because functions don't need custom matcher handling or support for advanced middleware - but do want trailing slash handling unlike middleware

// 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
Loading