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

fix: better dev routing with base using middleware #3942

Merged
merged 1 commit into from
Jul 21, 2022
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
5 changes: 5 additions & 0 deletions .changeset/dull-eagles-beg.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Use a base middleware for better base path handling in dev.
100 changes: 58 additions & 42 deletions packages/astro/src/vite-plugin-astro-server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,38 +111,14 @@ async function handle404Response(
req: http.IncomingMessage,
res: http.ServerResponse
) {
const site = config.site ? new URL(config.base, config.site) : undefined;
const devRoot = site ? site.pathname : '/';
const pathname = decodeURI(new URL(origin + req.url).pathname);
let html = '';
if (pathname === '/' && !pathname.startsWith(devRoot)) {
html = subpathNotUsedTemplate(devRoot, pathname);
} else {
// HACK: redirect without the base path for assets in publicDir
const redirectTo =
req.method === 'GET' &&
config.base !== '/' &&
pathname.startsWith(config.base) &&
pathname.replace(config.base, '/');

if (redirectTo && redirectTo !== '/') {
const response = new Response(null, {
status: 302,
headers: {
Location: redirectTo,
},
});
await writeWebResponse(res, response);
return;
}

html = notFoundTemplate({
statusCode: 404,
title: 'Not found',
tabTitle: '404: Not Found',
pathname,
});
}
const html = notFoundTemplate({
statusCode: 404,
title: 'Not found',
tabTitle: '404: Not Found',
pathname,
});
writeHtmlResponse(res, 404, html);
}

Expand Down Expand Up @@ -177,6 +153,44 @@ function log404(logging: LogOptions, pathname: string) {
info(logging, 'serve', msg.req({ url: pathname, statusCode: 404 }));
}

export function baseMiddleware(
config: AstroConfig,
logging: LogOptions
): vite.Connect.NextHandleFunction {
const site = config.site ? new URL(config.base, config.site) : undefined;
const devRoot = site ? site.pathname : '/';

return function devBaseMiddleware(req, res, next) {
const url = req.url!;

const pathname = decodeURI(new URL(url, 'http://vitejs.dev').pathname);

if (pathname.startsWith(devRoot)) {
req.url = url.replace(devRoot, '/');
return next();
}

if (pathname === '/' || pathname === '/index.html') {
log404(logging, pathname);
const html = subpathNotUsedTemplate(devRoot, pathname);
return writeHtmlResponse(res, 404, html);
}

if (req.headers.accept?.includes('text/html')) {
log404(logging, pathname);
const html = notFoundTemplate({
statusCode: 404,
title: 'Not found',
tabTitle: '404: Not Found',
pathname,
});
return writeHtmlResponse(res, 404, html);
}

next();
};
}

/** The main logic to route dev server requests to pages in Astro. */
async function handleRequest(
routeCache: RouteCache,
Expand All @@ -188,19 +202,19 @@ async function handleRequest(
res: http.ServerResponse
) {
const reqStart = performance.now();
const site = config.site ? new URL(config.base, config.site) : undefined;
const devRoot = site ? site.pathname : '/';
const origin = `${viteServer.config.server.https ? 'https' : 'http'}://${req.headers.host}`;
const buildingToSSR = isBuildingToSSR(config);
// Ignore `.html` extensions and `index.html` in request URLS to ensure that
// routing behavior matches production builds. This supports both file and directory
// build formats, and is necessary based on how the manifest tracks build targets.
const url = new URL(origin + req.url?.replace(/(index)?\.html$/, ''));
const pathname = decodeURI(url.pathname);
const rootRelativeUrl = pathname.substring(devRoot.length - 1);

// Add config.base back to url before passing it to SSR
url.pathname = config.base.substring(0, config.base.length - 1) + url.pathname;

// HACK! @astrojs/image uses query params for the injected route in `dev`
if (!buildingToSSR && rootRelativeUrl !== '/_image') {
if (!buildingToSSR && pathname !== '/_image') {
// Prevent user from depending on search params when not doing SSR.
// NOTE: Create an array copy here because deleting-while-iterating
// creates bugs where not all search params are removed.
Expand Down Expand Up @@ -232,13 +246,9 @@ async function handleRequest(
});

try {
if (!pathname.startsWith(devRoot)) {
log404(logging, pathname);
return handle404Response(origin, config, req, res);
}
// Attempt to match the URL to a valid page route.
// If that fails, switch the response to a 404 response.
let route = matchRoute(rootRelativeUrl, manifest);
let route = matchRoute(pathname, manifest);
const statusCode = route ? 200 : 404;

if (!route) {
Expand All @@ -260,7 +270,7 @@ async function handleRequest(
mod,
route,
routeCache,
pathname: rootRelativeUrl,
pathname: pathname,
logging,
ssr: isBuildingToSSR(config),
});
Expand All @@ -285,7 +295,7 @@ async function handleRequest(
logging,
mode: 'development',
origin,
pathname: rootRelativeUrl,
pathname: pathname,
request,
route: routeCustom404,
routeCache,
Expand All @@ -303,7 +313,7 @@ async function handleRequest(
logging,
mode: 'development',
origin,
pathname: rootRelativeUrl,
pathname: pathname,
route,
routeCache,
viteServer,
Expand Down Expand Up @@ -386,6 +396,12 @@ export default function createPlugin({ config, logging }: AstroPluginOptions): v
route: '',
handle: forceTextCSSForStylesMiddleware,
});
if (config.base !== '/') {
viteServer.middlewares.stack.unshift({
route: '',
handle: baseMiddleware(config, logging),
});
}
viteServer.middlewares.use(async (req, res) => {
if (!req.url || !req.method) {
throw new Error('Incomplete request');
Expand Down
8 changes: 8 additions & 0 deletions packages/astro/test/fixtures/public-base-404/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"name": "@test/public-base-404",
"version": "0.0.0",
"private": true,
"dependencies": {
"astro": "workspace:*"
}
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<html lang="en">
<head>
<title>Not Found</title>
</head>
<body>
<h1>404</h1>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<html lang="en">
<head>
<title>This Site</title>
</head>
<body>
<img src="/twitter.png" />
</body>
</html>
64 changes: 64 additions & 0 deletions packages/astro/test/public-base-404.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import { expect } from 'chai';
import * as cheerio from 'cheerio';
import { loadFixture } from './test-utils.js';

describe('Public dev with base', () => {
/** @type {import('./test-utils').Fixture} */
let fixture;
/** @type {import('./test-utils').DevServer} */
let devServer;
let $;

before(async () => {
fixture = await loadFixture({
root: './fixtures/public-base-404/',
site: 'http://example.com/',
base: '/blog'
});
devServer = await fixture.startDevServer();
});

it('200 when loading /@vite/client', async () => {
const response = await fixture.fetch('/@vite/client', {
redirect: 'manual'
});
expect(response.status).to.equal(200);
const content = await response.text()
expect(content).to.contain('vite')
});

it('200 when loading /blog/twitter.png', async () => {
const response = await fixture.fetch('/blog/twitter.png', {
redirect: 'manual'
});
expect(response.status).to.equal(200);
});

it('custom 404 page when loading /blog/blog/', async () => {
const response = await fixture.fetch('/blog/blog/');
const html = await response.text()
$ = cheerio.load(html);
expect($('h1').text()).to.equal('404');
});

it('default 404 hint page when loading /', async () => {
const response = await fixture.fetch('/');
expect(response.status).to.equal(404);
const html = await response.text()
$ = cheerio.load(html);
expect($('a').first().text()).to.equal('/blog/');
});

it('default 404 page when loading /none/', async () => {
const response = await fixture.fetch('/none/', {
headers: {
accept: 'text/html,*/*'
}
});
expect(response.status).to.equal(404);
const html = await response.text()
$ = cheerio.load(html);
expect($('h1').text()).to.equal('404: Not found');
expect($('pre').text()).to.equal('Path: /none/');
});
});
6 changes: 6 additions & 0 deletions pnpm-lock.yaml

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