Improvements to build and dev when building for subpaths (#3156)

* `astro build` should include the `base` provided in astro config

* test: updating build test to expect the base path in links/scripts

* ignore the default "base" value when building links/scripts

* fix: handling config that provides a base but no site

* fix: config.site was being ignored since it's a URL not a string

* hack: handle base path in dev for /public assets

* fix: dev redirect needs to ignore base default of ./

* fix: extra safety checks for the base path redirect

* refactor: simplifying it to remove the regex

* one last safety check - only redirect GET and use a 302 status

* fix: lost the leading slash when redirecting

* nit: adding comments to the test explaining how base is verified

* Remove extra console.log

* Adds a changeset

Co-authored-by: unknown <matthew@skypack.dev>
This commit is contained in:
Tony Sullivan 2022-04-21 18:03:05 +00:00 committed by GitHub
parent 0406bdc35b
commit 637919c8b6
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 51 additions and 3 deletions

View file

@ -0,0 +1,5 @@
---
'astro': patch
---
Adds subpath to assets/scripts when statically generating

View file

@ -11,7 +11,7 @@ import type {
} from '../../@types/astro';
import type { BuildInternals } from '../../core/build/internal.js';
import { debug, info } from '../logger/core.js';
import { prependForwardSlash, removeLeadingForwardSlash } from '../../core/path.js';
import { joinPaths, prependForwardSlash, removeLeadingForwardSlash } from '../../core/path.js';
import type { RenderOptions } from '../../core/render/core';
import { BEFORE_HYDRATION_SCRIPT_ID } from '../../vite-plugin-scripts/index.js';
import { call as callEndpoint } from '../endpoint/index.js';
@ -176,7 +176,11 @@ async function generatePath(
debug('build', `Generating: ${pathname}`);
const site = astroConfig.site;
// If a base path was provided, append it to the site URL. This ensures that
// all injected scripts and links are referenced relative to the site and subpath.
const site = astroConfig.base && astroConfig.base !== './'
? joinPaths(astroConfig.site?.toString() || 'http://localhost/', astroConfig.base)
: astroConfig.site;
const links = createLinkStylesheetElementSet(linkIds.reverse(), site);
const scripts = createModuleScriptElementWithSrcSet(hoistedId ? [hoistedId] : [], site);

View file

@ -38,3 +38,11 @@ export function startsWithDotSlash(path: string) {
export function isRelativePath(path: string) {
return startsWithDotDotSlash(path) || startsWithDotSlash(path);
}
function isString(path: unknown): path is string {
return typeof path === 'string' || path instanceof String;
}
export function joinPaths(...paths: (string | undefined)[]) {
return paths.filter(isString).map(trimSlashes).join('/');
}

View file

@ -111,6 +111,24 @@ async function handle404Response(
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 && 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',

View file

@ -6,6 +6,11 @@ function addLeadingSlash(path) {
return path.startsWith('/') ? path : '/' + path;
}
function removeBasePath(path) {
// `/subpath` is defined in the test fixture's Astro config
return path.replace('/subpath', '')
}
/**
* @typedef {import('../src/core/logger/core').LogMessage} LogMessage
*/
@ -90,7 +95,15 @@ describe('Static build', () => {
const links = $('link[rel=stylesheet]');
for (const link of links) {
const href = $(link).attr('href');
const data = await fixture.readFile(addLeadingSlash(href));
/**
* The link should be built with the config's `base` included
* as a subpath.
*
* The test needs to verify that the file will be found once the `/dist`
* output is deployed to a subpath in production by ignoring the subpath here.
*/
const data = await fixture.readFile(removeBasePath(addLeadingSlash(href)));
if (expected.test(data)) {
return true;
}