Make Vercel URL rewrites precise (#2913)

### The problem

Right now we use a catchall path in Vercel routing config to rewrite all
requests that don't match existing assets to `/index.html`, which is
needed for client side routing to work. This, however, messes up 404
errors for truly non-existing files which won't be handled by the SPA,
because they get redirected to index.html.

Even worse, this interacts very poorly with caching. Normally if we
request a non-existent file, then put the file in place, and request the
file again, we'll get 404 the first time and the actual file the second
time. However, in our case we instead return `/index.html` after the
first attempt and cache that response, making it impossible to correct a
missing file without cache flush.

### The solution

One way to fix this is to make the regex in Vercel config precise, so
that they only match our SPA routes. However, it can be dangerous,
because this means we'll need to manually update the config with new SPA
routes every time we add any. This PR tests that regexes we're using in
Vercel match all routes that we set in the SPA router.

### Potential future improvements

It's very possible to generate Vercel's config from React Router routing
objects, but at the moment it's not done because that would require
importing most of dotcom during the build phase, which seem to cause
errors.

### Change Type

- [x] `internal` — Any other changes that don't affect the published
package[^2]

### Test Plan

1. Might need a light smoke test after deployment to dotcom.

- [x] End to end tests
This commit is contained in:
Dan Groshev 2024-02-22 18:25:45 +00:00 committed by GitHub
parent eb3706e918
commit a8999aa0a0
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 143 additions and 69 deletions

View file

@ -36,6 +36,7 @@
"react-router-dom": "^6.17.0"
},
"devDependencies": {
"@jest/globals": "30.0.0-alpha.2",
"@sentry/cli": "^2.25.0",
"@types/qrcode": "^1.5.0",
"@types/react": "^18.2.47",

View file

@ -5,6 +5,8 @@ import { Config } from './vercel-output-config'
import { config } from 'dotenv'
import { nicelog } from '../../../scripts/lib/nicelog'
import { SPA_ROUTE_FILTERS } from '../spaRouteFilters'
config({
path: './.env.local',
})
@ -19,6 +21,13 @@ async function build() {
mkdirSync('.vercel/output', { recursive: true })
await exec('cp', ['-r', 'dist', '.vercel/output/static'])
await exec('rm', ['-rf', ...glob.sync('.vercel/output/static/**/*.js.map')])
const spaRoutes = SPA_ROUTE_FILTERS.map((route) => ({
check: true,
src: route,
dest: '/index.html',
}))
writeFileSync(
'.vercel/output/config.json',
JSON.stringify(
@ -43,10 +52,13 @@ async function build() {
handle: 'miss',
},
// finally handle SPA routing
...spaRoutes,
// react router will handle drawing the 404 page
{
check: true,
src: '.*',
dest: '/index.html',
status: 404,
},
],
overrides: {},

View file

@ -0,0 +1,8 @@
// This value is used to configure Vercel routing to rewrite dotcom SPA routes to /index.html.
// It is also tested in routes.test.ts to make sure it matches all React Router routes.
//
// It is a list of string-encoded regexes matching SPA routes to be spliced into
// Vercel's "build output spec" in scripts/build.ts.
//
// Make sure it's not overly broad, because otherwise we won't give correct 404 responses.
export const SPA_ROUTE_FILTERS = ['^/$', '^/r/?$', '^/new/?$', '^/r/.*', '^/s/.*', '^/v/.*']

View file

@ -2,7 +2,6 @@ import { captureException } from '@sentry/react'
import { DefaultErrorFallback as ErrorFallback } from '@tldraw/tldraw'
import { useEffect } from 'react'
import { useRouteError } from 'react-router-dom'
import '../../../styles/globals.css'
export function DefaultErrorFallback() {
const error = useRouteError()

View file

@ -1,82 +1,19 @@
import { captureException } from '@sentry/react'
import { Analytics as VercelAnalytics } from '@vercel/analytics/react'
import { nanoid } from 'nanoid'
import { useEffect } from 'react'
import { createRoot } from 'react-dom/client'
import { HelmetProvider } from 'react-helmet-async'
import {
Outlet,
Route,
RouterProvider,
createBrowserRouter,
createRoutesFromElements,
redirect,
useRouteError,
} from 'react-router-dom'
import { RouterProvider, createBrowserRouter } from 'react-router-dom'
import '../sentry.client.config'
import '../styles/core.css'
import { DefaultErrorFallback } from './components/DefaultErrorFallback/DefaultErrorFallback'
import { ErrorPage } from './components/ErrorPage/ErrorPage'
import '../styles/globals.css'
import { Head } from './components/Head/Head'
import { router } from './routes'
const router = createBrowserRouter(
createRoutesFromElements(
<Route
element={
// Add all top level providers that require the router here
<Outlet />
}
ErrorBoundary={() => {
const error = useRouteError()
useEffect(() => {
captureException(error)
}, [error])
return (
<ErrorPage
messages={{
header: 'Something went wrong',
para1:
'Please try refreshing the page. Still having trouble? Let us know at hello@tldraw.com.',
}}
/>
)
}}
>
<Route errorElement={<DefaultErrorFallback />}>
<Route path="/" lazy={() => import('./pages/root')} />
<Route
path="/r"
loader={() => {
const id = 'v2' + nanoid()
return redirect(`/r/${id}`)
}}
/>
<Route
path="/new"
loader={() => {
const id = 'v2' + nanoid()
return redirect(`/r/${id}`)
}}
/>
<Route path="/r/:roomId" lazy={() => import('./pages/public-multiplayer')} />
<Route path="/r/:boardId/history" lazy={() => import('./pages/history')} />
<Route
path="/r/:boardId/history/:timestamp"
lazy={() => import('./pages/history-snapshot')}
/>
<Route path="/s/:roomId" lazy={() => import('./pages/public-snapshot')} />
<Route path="/v/:roomId" lazy={() => import('./pages/public-readonly')} />
</Route>
<Route path="*" lazy={() => import('./pages/not-found')} />
</Route>
)
)
const browserRouter = createBrowserRouter(router)
createRoot(document.getElementById('root')!).render(
<HelmetProvider>
<Head />
<RouterProvider router={router} />
<RouterProvider router={browserRouter} />
<VercelAnalytics debug={false} />
</HelmetProvider>
)

View file

@ -0,0 +1,58 @@
import { expect } from '@jest/globals'
import type { MatcherFunction } from 'expect'
import { RouteObject } from 'react-router-dom'
import { SPA_ROUTE_FILTERS } from '../spaRouteFilters'
import { router } from './routes'
const toMatchAny: MatcherFunction<[regexes: unknown]> = function (actual, regexes) {
if (
typeof actual !== 'string' ||
!Array.isArray(regexes) ||
regexes.some((regex) => typeof regex !== 'string')
) {
throw new Error('toMatchAny takes a string and an array of strings')
}
const pass = regexes.map((regex) => new RegExp(regex)).some((regex) => actual.match(regex))
if (pass) {
return {
message: () =>
`expected ${this.utils.printReceived(actual)} not to match any of the regexes ${this.utils.printExpected(regexes)}`,
pass: true,
}
} else {
return {
message: () =>
`expected ${this.utils.printReceived(actual)} to match at least one of the regexes ${this.utils.printExpected(regexes)}`,
pass: false,
}
}
}
expect.extend({ toMatchAny })
function extractContentPaths(routeObject: RouteObject): string[] {
const paths: string[] = []
if (routeObject.path && routeObject.path !== '*') {
paths.push(routeObject.path)
}
if (routeObject.children) {
routeObject.children.forEach((child) => {
paths.push(...extractContentPaths(child))
})
}
return paths
}
test('SPA_ROUTE_FILTERS match all React routes', () => {
router.flatMap(extractContentPaths).forEach((path) => {
expect(path).toMatchAny(SPA_ROUTE_FILTERS)
})
})
test("SPA_ROUTE_FILTERS don't match assets", () => {
expect('/assets/test.png').not.toMatchAny(SPA_ROUTE_FILTERS)
})

View file

@ -0,0 +1,58 @@
import { captureException } from '@sentry/react'
import { nanoid } from 'nanoid'
import { useEffect } from 'react'
import { createRoutesFromElements, Outlet, redirect, Route, useRouteError } from 'react-router-dom'
import { DefaultErrorFallback } from './components/DefaultErrorFallback/DefaultErrorFallback'
import { ErrorPage } from './components/ErrorPage/ErrorPage'
export const router = createRoutesFromElements(
<Route
element={
// Add all top level providers that require the router here
<Outlet />
}
ErrorBoundary={() => {
const error = useRouteError()
useEffect(() => {
captureException(error)
}, [error])
return (
<ErrorPage
messages={{
header: 'Something went wrong',
para1:
'Please try refreshing the page. Still having trouble? Let us know at hello@tldraw.com.',
}}
/>
)
}}
>
<Route errorElement={<DefaultErrorFallback />}>
<Route path="/" lazy={() => import('./pages/root')} />
<Route
path="/r"
loader={() => {
const id = 'v2' + nanoid()
return redirect(`/r/${id}`)
}}
/>
<Route
path="/new"
loader={() => {
const id = 'v2' + nanoid()
return redirect(`/r/${id}`)
}}
/>
<Route path="/r/:roomId" lazy={() => import('./pages/public-multiplayer')} />
<Route path="/r/:boardId/history" lazy={() => import('./pages/history')} />
<Route
path="/r/:boardId/history/:timestamp"
lazy={() => import('./pages/history-snapshot')}
/>
<Route path="/s/:roomId" lazy={() => import('./pages/public-snapshot')} />
<Route path="/v/:roomId" lazy={() => import('./pages/public-readonly')} />
</Route>
<Route path="*" lazy={() => import('./pages/not-found')} />
</Route>
)

View file

@ -11942,6 +11942,7 @@ __metadata:
version: 0.0.0-use.local
resolution: "dotcom@workspace:apps/dotcom"
dependencies:
"@jest/globals": "npm:30.0.0-alpha.2"
"@radix-ui/react-popover": "npm:^1.0.7"
"@sentry/cli": "npm:^2.25.0"
"@sentry/integrations": "npm:^7.34.0"