From 27cc29ffd047f784a432512de45e1064b8f866cb Mon Sep 17 00:00:00 2001 From: Florian Lefebvre Date: Mon, 12 May 2025 10:18:48 +0200 Subject: [PATCH 1/8] fix(fonts): take build config into account --- packages/astro/src/assets/fonts/constants.ts | 3 +-- .../assets/fonts/implementations/url-proxy.ts | 6 ++--- .../src/assets/fonts/vite-plugin-fonts.ts | 24 ++++++++++++------- 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/packages/astro/src/assets/fonts/constants.ts b/packages/astro/src/assets/fonts/constants.ts index 09beec38245a..33993ef9c0b3 100644 --- a/packages/astro/src/assets/fonts/constants.ts +++ b/packages/astro/src/assets/fonts/constants.ts @@ -14,8 +14,7 @@ export const DEFAULTS: Defaults = { export const VIRTUAL_MODULE_ID = 'virtual:astro:assets/fonts/internal'; export const RESOLVED_VIRTUAL_MODULE_ID = '\0' + VIRTUAL_MODULE_ID; -// Requires a trailing slash -export const URL_PREFIX = '/_astro/fonts/'; +export const ASSETS_DIR = 'fonts'; export const CACHE_DIR = './fonts/'; export const FONT_TYPES = ['woff2', 'woff', 'otf', 'ttf', 'eot'] as const; diff --git a/packages/astro/src/assets/fonts/implementations/url-proxy.ts b/packages/astro/src/assets/fonts/implementations/url-proxy.ts index a12e7b986f76..9c539c6f6490 100644 --- a/packages/astro/src/assets/fonts/implementations/url-proxy.ts +++ b/packages/astro/src/assets/fonts/implementations/url-proxy.ts @@ -1,20 +1,20 @@ import type { DataCollector, Hasher, UrlProxy, UrlProxyContentResolver } from '../definitions.js'; export function createUrlProxy({ - base, contentResolver, hasher, dataCollector, + getUrl, }: { - base: string; contentResolver: UrlProxyContentResolver; hasher: Hasher; dataCollector: DataCollector; + getUrl: (hash: string) => string; }): UrlProxy { return { proxy({ url: originalUrl, type, data, collectPreload, init }) { const hash = `${hasher.hashString(contentResolver.resolve(originalUrl))}.${type}`; - const url = base + hash; + const url = getUrl(hash); dataCollector.collect({ url: originalUrl, diff --git a/packages/astro/src/assets/fonts/vite-plugin-fonts.ts b/packages/astro/src/assets/fonts/vite-plugin-fonts.ts index 44ebcd6f012e..37db0254eccf 100644 --- a/packages/astro/src/assets/fonts/vite-plugin-fonts.ts +++ b/packages/astro/src/assets/fonts/vite-plugin-fonts.ts @@ -2,7 +2,6 @@ import { mkdirSync, writeFileSync } from 'node:fs'; import { readFile } from 'node:fs/promises'; import { isAbsolute } from 'node:path'; import { fileURLToPath } from 'node:url'; -import { removeTrailingForwardSlash } from '@astrojs/internal-helpers/path'; import type { Plugin } from 'vite'; import { collectErrorMetadata } from '../../core/errors/dev/utils.js'; import { AstroError, AstroErrorData, isAstroError } from '../../core/errors/index.js'; @@ -11,10 +10,10 @@ import { formatErrorMessage } from '../../core/messages.js'; import { getClientOutputDirectory } from '../../prerender/utils.js'; import type { AstroSettings } from '../../types/astro.js'; import { + ASSETS_DIR, CACHE_DIR, DEFAULTS, RESOLVED_VIRTUAL_MODULE_ID, - URL_PREFIX, VIRTUAL_MODULE_ID, } from './constants.js'; import type { @@ -46,6 +45,8 @@ import { import { createUrlProxy } from './implementations/url-proxy.js'; import { orchestrate } from './orchestrate.js'; import type { ConsumableMap, FontFileDataMap } from './types.js'; +import { joinPaths, prependForwardSlash } from '../../core/path.js'; +import { getAssetsPrefix } from '../utils/getAssetsPrefix.js'; interface Options { settings: AstroSettings; @@ -78,7 +79,8 @@ export function fontsPlugin({ settings, sync, logger }: Options): Plugin { // We don't need to take the trailing slash and build output configuration options // into account because we only serve (dev) or write (build) static assets (equivalent // to trailingSlash: never) - const baseUrl = removeTrailingForwardSlash(settings.config.base) + URL_PREFIX; + const assetsDir = joinPaths(settings.config.build.assets, ASSETS_DIR); + const baseUrl = joinPaths(settings.config.base, assetsDir); let fontFileDataMap: FontFileDataMap | null = null; let consumableMap: ConsumableMap | null = null; @@ -153,7 +155,15 @@ export function fontsPlugin({ settings, sync, logger }: Options): Plugin { ? createLocalUrlProxyContentResolver({ errorHandler }) : createRemoteUrlProxyContentResolver(); return createUrlProxy({ - base: baseUrl, + getUrl: (hash) => { + const prefix = settings.config.build.assetsPrefix + ? getAssetsPrefix(hash, settings.config.build.assetsPrefix) + : undefined; + if (prefix) { + return joinPaths(prefix, baseUrl, hash); + } + return prependForwardSlash(joinPaths(baseUrl, hash)); + }, contentResolver, hasher, dataCollector, @@ -209,9 +219,7 @@ export function fontsPlugin({ settings, sync, logger }: Options): Plugin { } }); - // Base is taken into account by default. The prefix contains a traling slash, - // so it matches correctly any hash, eg. /_astro/fonts/abc.woff => abc.woff - server.middlewares.use(URL_PREFIX, async (req, res, next) => { + server.middlewares.use(prependForwardSlash(assetsDir), async (req, res, next) => { if (!req.url) { return next(); } @@ -269,7 +277,7 @@ export function fontsPlugin({ settings, sync, logger }: Options): Plugin { try { const dir = getClientOutputDirectory(settings); - const fontsDir = new URL('.' + baseUrl, dir); + const fontsDir = new URL(`.${prependForwardSlash(baseUrl)}`, dir); try { mkdirSync(fontsDir, { recursive: true }); } catch (cause) { From 00102d4242159ad74984d1b70315cf643766e3b1 Mon Sep 17 00:00:00 2001 From: Florian Lefebvre Date: Mon, 12 May 2025 10:35:39 +0200 Subject: [PATCH 2/8] fix: test --- packages/astro/test/units/assets/fonts/orchestrate.test.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/astro/test/units/assets/fonts/orchestrate.test.js b/packages/astro/test/units/assets/fonts/orchestrate.test.js index c50cf5190d8b..07018f3bb7e4 100644 --- a/packages/astro/test/units/assets/fonts/orchestrate.test.js +++ b/packages/astro/test/units/assets/fonts/orchestrate.test.js @@ -22,6 +22,7 @@ import { fakeHasher, simpleErrorHandler, } from './utils.js'; +import { defaultLogger } from '../../test-utils.js'; describe('fonts orchestrate()', () => { it('works with local fonts', async () => { @@ -58,11 +59,12 @@ describe('fonts orchestrate()', () => { fontMetricsResolver: fakeFontMetricsResolver, fontTypeExtractor, fontFileReader: createFontaceFontFileReader({ errorHandler }), + logger: defaultLogger, createUrlProxy: ({ local, ...params }) => { const dataCollector = createDataCollector(params); const contentResolver = createRemoteUrlProxyContentResolver(); return createUrlProxy({ - base: '/test', + getUrl: (hash) => `/test${hash}`, contentResolver, hasher, dataCollector, @@ -158,11 +160,12 @@ describe('fonts orchestrate()', () => { fontMetricsResolver: fakeFontMetricsResolver, fontTypeExtractor, fontFileReader: createFontaceFontFileReader({ errorHandler }), + logger: defaultLogger, createUrlProxy: ({ local, ...params }) => { const dataCollector = createDataCollector(params); const contentResolver = createRemoteUrlProxyContentResolver(); return createUrlProxy({ - base: '', + getUrl: (hash) => hash, contentResolver, hasher, dataCollector, From 09ad2eb4ce18ca62120ae5dd5ebe2cf2757d0959 Mon Sep 17 00:00:00 2001 From: Florian Lefebvre Date: Mon, 12 May 2025 11:08:20 +0200 Subject: [PATCH 3/8] feat: refactor fixture test --- packages/astro/test/fonts.test.js | 176 +++++++++++++++++++++--------- 1 file changed, 124 insertions(+), 52 deletions(-) diff --git a/packages/astro/test/fonts.test.js b/packages/astro/test/fonts.test.js index 47faef1c4510..a9d0c1694fc6 100644 --- a/packages/astro/test/fonts.test.js +++ b/packages/astro/test/fonts.test.js @@ -1,76 +1,110 @@ // @ts-check import assert from 'node:assert/strict'; -import { after, before, describe, it } from 'node:test'; +import { describe, it } from 'node:test'; import { fontProviders } from 'astro/config'; import * as cheerio from 'cheerio'; import { loadFixture } from './test-utils.js'; -describe('astro:fonts', () => { - /** @type {import('./test-utils.js').Fixture} */ - let fixture; - /** @type {import('./test-utils.js').DevServer} */ - let devServer; +/** + * @param {Omit} inlineConfig + */ +async function createDevFixture(inlineConfig) { + const fixture = await loadFixture({ root: './fixtures/fonts/', ...inlineConfig }); + const devServer = await fixture.startDevServer(); - describe(' component', () => { - // TODO: remove once fonts are stabilized - describe('Fonts are not enabled', () => { - before(async () => { - fixture = await loadFixture({ - root: './fixtures/fonts/', - }); - devServer = await fixture.startDevServer(); - }); - - after(async () => { + return { + fixture, + devServer, + run: async (/** @type {() => any} */ cb) => { + try { + return await cb(); + } finally { await devServer.stop(); - }); + } + }, + }; +} +/** + * @param {Omit} inlineConfig + */ +async function createBuildFixture(inlineConfig) { + const fixture = await loadFixture({ root: './fixtures/fonts/', ...inlineConfig }); + await fixture.build({}); - it('Throws an error if fonts are not enabled', async () => { - const res = await fixture.fetch('/'); - const body = await res.text(); - assert.equal( - body.includes('