Skip to content
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

add util for handling remix headers generally #810

Merged
merged 5 commits into from
Jan 17, 2025
Merged
Show file tree
Hide file tree
Changes from 3 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
8 changes: 2 additions & 6 deletions app/root.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import { getEnv } from './utils/env.server.ts'
import { honeypot } from './utils/honeypot.server.ts'
import { combineHeaders, getDomainUrl, getUserImgSrc } from './utils/misc.tsx'
import { useNonce } from './utils/nonce-provider.ts'
import { pipeHeaders } from './utils/remix.server.ts'
import { type Theme, getTheme } from './utils/theme.server.ts'
import { makeTimings, time } from './utils/timing.server.ts'
import { getToast } from './utils/toast.server.ts'
Expand Down Expand Up @@ -142,12 +143,7 @@ export async function loader({ request }: LoaderFunctionArgs) {
)
}

export const headers: HeadersFunction = ({ loaderHeaders }) => {
const headers = {
'Server-Timing': loaderHeaders.get('Server-Timing') ?? '',
}
return headers
}
export const headers: HeadersFunction = pipeHeaders

function Document({
children,
Expand Down
8 changes: 2 additions & 6 deletions app/routes/settings+/profile.connections.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
providerNames,
} from '#app/utils/connections.tsx'
import { prisma } from '#app/utils/db.server.ts'
import { pipeHeaders } from '#app/utils/remix.server.js'
import { makeTimings } from '#app/utils/timing.server.ts'
import { createToastHeaders } from '#app/utils/toast.server.ts'
import { type Info, type Route } from './+types/profile.connections.ts'
Expand Down Expand Up @@ -84,12 +85,7 @@ export async function loader({ request }: Route.LoaderArgs) {
)
}

export const headers: Route.HeadersFunction = ({ loaderHeaders }) => {
const headers = {
'Server-Timing': loaderHeaders.get('Server-Timing') ?? '',
}
return headers
}
export const headers: Route.HeadersFunction = pipeHeaders

export async function action({ request }: Route.ActionArgs) {
const userId = await requireUserId(request)
Expand Down
41 changes: 41 additions & 0 deletions app/utils/remix.server.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { format, parse } from '@tusbar/cache-control'
import { describe, expect, test } from 'vitest'
import { getConservativeCacheControl } from './remix.server.ts'

describe('getConservativeCacheControl', () => {
Copy link
Member

Choose a reason for hiding this comment

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

I prefer to not use describe because it's unnecessary. Just put the test blocks at the root of the file.

test('works for basic usecase', () => {
const result = getConservativeCacheControl(
'max-age=3600',
'max-age=1800, s-maxage=600',
'private, max-age=86400',
)

expect(result).toEqual(
format({
maxAge: 1800,
sharedMaxAge: 600,
private: true,
}),
)
})
test('retains boolean directive', () => {
const result = parse(
getConservativeCacheControl('private', 'no-cache,no-store'),
)

expect(result.private).toEqual(true)
expect(result.noCache).toEqual(true)
expect(result.noStore).toEqual(true)
})
test('gets smallest number directive', () => {
const result = parse(
getConservativeCacheControl(
'max-age=10, s-maxage=300',
'max-age=300, s-maxage=600',
),
)

expect(result.maxAge).toEqual(10)
expect(result.sharedMaxAge).toEqual(300)
})
})
103 changes: 103 additions & 0 deletions app/utils/remix.server.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
import { type CacheControlValue, parse, format } from '@tusbar/cache-control'
Copy link
Member

Choose a reason for hiding this comment

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

I think calling this file remix.server.ts may not be the right call anymore. Maybe this should just be called headers.server.ts.

Also, could you add some jsdoc comments to these utilities? Thanks!

import { type HeadersArgs } from 'react-router'

export function pipeHeaders({
parentHeaders,
loaderHeaders,
actionHeaders,
errorHeaders,
}: HeadersArgs) {
const headers = new Headers()

// get the one that's actually in use
let currentHeaders: Headers
if (errorHeaders !== undefined) {
currentHeaders = errorHeaders
} else if (loaderHeaders.entries().next().done) {
currentHeaders = actionHeaders
} else {
currentHeaders = loaderHeaders
}

// take in useful headers route loader/action
// pass this point currentHeaders can be ignored
const forwardHeaders = ['Cache-Control', 'Vary', 'Server-Timing']
for (const headerName of forwardHeaders) {
const header = currentHeaders.get(headerName)
if (header) {
headers.set(headerName, header)
}
}

headers.set(
'Cache-Control',
getConservativeCacheControl(
parentHeaders.get('Cache-Control'),
headers.get('Cache-Control'),
),
)

// append useful parent headers
const inheritHeaders = ['Vary', 'Server-Timing']
for (const headerName of inheritHeaders) {
const header = parentHeaders.get(headerName)
if (header) {
headers.append(headerName, header)
}
}

// fallback to parent headers if loader don't have
const fallbackHeaders = ['Cache-Control', 'Vary']
for (const headerName of fallbackHeaders) {
if (headers.has(headerName)) {
continue
}
const fallbackHeader = parentHeaders.get(headerName)
if (fallbackHeader) {
headers.set(headerName, fallbackHeader)
}
}

return headers
}

export function getConservativeCacheControl(
...cacheControlHeaders: Array<string | null>
): string {
return format(
cacheControlHeaders
.filter(Boolean)
.map((header) => parse(header))
.reduce<CacheControlValue>((acc, current) => {
for (const key in current) {
const directive = key as keyof Required<CacheControlValue> // keyof CacheControl includes functions

const currentValue = current[directive]

switch (typeof currentValue) {
case 'boolean': {
if (currentValue) {
acc[directive] = true as any
}

break
}
case 'number': {
const accValue = acc[directive] as number | undefined

if (accValue === undefined) {
acc[directive] = currentValue as any
} else {
const result = Math.min(accValue, currentValue)
acc[directive] = result as any
}

break
}
}
}

return acc
}, {}),
)
}
2 changes: 2 additions & 0 deletions docs/server-timing.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ export async function loader({ params }: LoaderFunctionArgs) {
)
}

// We have a general headers handler to save you from boilerplating.
// export const headers: HeadersFunction = pipeHeaders
export const headers: HeadersFunction = ({ loaderHeaders, parentHeaders }) => {
return {
'Server-Timing': combineServerTimings(parentHeaders, loaderHeaders), // <-- 4. Send headers
Expand Down
6 changes: 6 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
"@sentry/node": "^8.47.0",
"@sentry/profiling-node": "^8.47.0",
"@sentry/react": "^8.47.0",
"@tusbar/cache-control": "1.0.2",
"address": "^2.0.3",
"bcryptjs": "^2.4.3",
"better-sqlite3": "^11.7.0",
Expand Down