Skip to content

Commit

Permalink
Improvements to build and dev when building for subpaths (#3156)
Browse files Browse the repository at this point in the history
* `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 <[email protected]>
  • Loading branch information
Tony Sullivan and matthewp authored Apr 21, 2022
1 parent 0406bdc commit 637919c
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 3 deletions.
5 changes: 5 additions & 0 deletions .changeset/chatty-peas-warn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Adds subpath to assets/scripts when statically generating
8 changes: 6 additions & 2 deletions packages/astro/src/core/build/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);

Expand Down
8 changes: 8 additions & 0 deletions packages/astro/src/core/path.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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('/');
}
18 changes: 18 additions & 0 deletions packages/astro/src/vite-plugin-astro-server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
15 changes: 14 additions & 1 deletion packages/astro/test/static-build.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -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;
}
Expand Down

0 comments on commit 637919c

Please sign in to comment.