diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c49188c8545..839dc97baa4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,19 @@ ## Unreleased +- fix(serve): Allow periods in most paths (#10114) + + Partial fix for route paths with periods in them. + + It's only "partial" because it doesn't fix it for `yarn rw dev`, as that's a + Vite bug + ([vitejs/vite#2415 (comment)](https://github.com/vitejs/vite/issues/2415#issuecomment-1720814355)). + And there's also an edge case for yarn rw serve where this doesn't fully + handle client-side routes that start with /assets/ and that also have a + last-segment that accepts a period, like /assets/client-route-image.jpg + + Fixes #9969 + - fix(deps): update prisma monorepo to v5.10.2 (#10088) This release updates Prisma to v5.10.2. Here are quick links to all the release notes since the last version (v5.9.1): diff --git a/packages/adapters/fastify/web/src/web.test.ts b/packages/adapters/fastify/web/src/web.test.ts index 7e49bb4720f8..0f3af359396d 100644 --- a/packages/adapters/fastify/web/src/web.test.ts +++ b/packages/adapters/fastify/web/src/web.test.ts @@ -1,6 +1,7 @@ -import fs from 'fs' -import path from 'path' +import * as fs from 'fs' +import * as path from 'path' +import type { FastifyInstance } from 'fastify' import Fastify from 'fastify' import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest' @@ -8,7 +9,7 @@ import { getPaths } from '@redwoodjs/project-config' import { redwoodFastifyWeb } from './web' -let original_RWJS_CWD +let original_RWJS_CWD: string beforeAll(() => { original_RWJS_CWD = process.env.RWJS_CWD @@ -24,7 +25,7 @@ describe('redwoodFastifyWeb', () => { console.log = vi.fn() // Set up and teardown the fastify instance with options. - let fastifyInstance + let fastifyInstance: FastifyInstance const port = 8910 @@ -248,16 +249,56 @@ describe('redwoodFastifyWeb', () => { }) describe("returns a 404 for assets that can't be found", () => { - it("returns a 404 for non-html assets that can't be found", async () => { + it("returns a 404 for assets that can't be found", async () => { const res = await fastifyInstance.inject({ method: 'GET', - url: '/kittens.png', + url: '/assets/kittens.png', }) expect(res.statusCode).toBe(404) }) - it('handles "."s in routes', async () => { + // This is testing current behavior - not ideal behavior. Feel free to + // update this test if you change the behavior. + // It's for the (hopefully rare) case where someone has a client-side + // route for /assets + it('returns a 200 for plain files, even in /assets/', async () => { + const res = await fastifyInstance.inject({ + method: 'GET', + url: '/assets/kittens', + }) + + expect(res.statusCode).toBe(200) + }) + + it('handles "."s in route segments', async () => { + const res = await fastifyInstance.inject({ + method: 'GET', + url: '/my.page/foo', + }) + + expect(res.statusCode).toBe(200) + }) + + it('handles "."s in last route segment', async () => { + const res = await fastifyInstance.inject({ + method: 'GET', + url: '/foo/my.page', + }) + + expect(res.statusCode).toBe(200) + }) + + it('handles filenames in route segments', async () => { + const res = await fastifyInstance.inject({ + method: 'GET', + url: '/file-route/fake.js', + }) + + expect(res.statusCode).toBe(200) + }) + + it('handles "."s in query params', async () => { const res = await fastifyInstance.inject({ method: 'GET', url: '/my-page?loading=spinner.blue', diff --git a/packages/adapters/fastify/web/src/web.ts b/packages/adapters/fastify/web/src/web.ts index ac5e25b64f8e..d5268d58a5dd 100644 --- a/packages/adapters/fastify/web/src/web.ts +++ b/packages/adapters/fastify/web/src/web.ts @@ -84,19 +84,36 @@ export async function redwoodFastifyWeb( // For SPA routing, fallback on unmatched routes and let client-side routing take over fastify.setNotFoundHandler({}, (req, reply) => { const urlData = req.urlData() - const requestedExtension = path.extname(urlData.path ?? '') - - // Paths with no extension (`/about`) or an .html extension (`/about.html`) - // should be handled by the client side router. - // See the discussion in https://github.com/redwoodjs/redwood/pull/9272. - if (requestedExtension === '' || requestedExtension === '.html') { - reply.header('Content-Type', 'text/html; charset=UTF-8') - return reply.sendFile(fallbackIndexPath) + const requestHasExtension = !!path.extname(urlData.path ?? '') + + // Further up in this file we use `fastifyStatic` to serve files from the + // /web/dist folder. Most often for files like AboutPage-12ab34cd.js or + // some css file. + // Requests for other paths should most often be handled by client side + // routing. Like requests /about or /about.html. + // One exception for this is requests for assets that don't exist anymore. + // Like AboutPage-old_hash.js. These requests should return 404. + // The problem is we don't know what those assets are. So the best we can + // do is to return 404 for all requests for files in /assets that have an + // extension. + // + // See the discussions in https://github.com/redwoodjs/redwood/pull/9272 + // and https://github.com/redwoodjs/redwood/issues/9969 + + if (requestHasExtension && urlData.path?.startsWith('/assets/')) { + // If we got here, the user is most likely requesting an asset with an + // extension (like `assets/AboutPage-xyz789.js`) that doesn't exist + // + // NOTE: This is a best guess, and could be wrong. The user could have + // a client-side route setup for /assets/client-side/{...} and in that + // case we really should pass this on to the client-side router instead + // of returning 404. + reply.code(404) + return reply.send('Not Found') } - // If we got here, the user is requesting an asset with an extension - // (like `profile.png`) that doesn't exist - reply.code(404) - return reply.send('Not Found') + // Let client-side routing take over + reply.header('Content-Type', 'text/html; charset=UTF-8') + return reply.sendFile(fallbackIndexPath) }) }