Skip to content

✨ feat(font): add assetsPrefix support for CDN deployments #30

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.

Sign up for GitHub

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

Open
jcdogo wants to merge 1 commit into
base: master
Choose a base branch
from
Open

✨ feat(font): add assetsPrefix support for CDN deployments #30

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 14 additions & 3 deletions packages/astro-font/AstroFont.astro
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,21 @@
---
import type { Props } from './dist/utils'
import type { Config } from './dist/utils'
import { generateFonts, getPreloadType, createPreloads, createBaseCSS, createFontCSS } from './dist/utils'

const { config } = Astro.props as Props
interface Props {
config: Config[];
assetsPrefix?: string;
}
Comment on lines +5 to +8

Copilot AI Feb 17, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Props interface is redefined locally in this file instead of extending the existing Props interface from utils.ts. This creates a discrepancy where the utils.ts exports Props with only a config property, but this file defines a different Props interface with both config and assetsPrefix. This could cause confusion and type inconsistencies. Consider either updating the exported Props interface in utils.ts to include the optional assetsPrefix property, or using a different name for this local interface (e.g., AstroFontProps) to avoid shadowing the exported type.

Copilot uses AI. Check for mistakes.

const resolvedConfig = await generateFonts(config)
const { config: originalConfig, assetsPrefix } = Astro.props as Props

const configWithPrefix: Config[] = originalConfig.map(conf => ({
...conf,
// Add the assetsPrefix passed to the component to each font config
assetsPrefix: assetsPrefix,
Comment on lines +14 to +15

Copilot AI Feb 17, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When assetsPrefix is undefined, the spread operator on line 15 will add assetsPrefix: undefined to each config object. While this works functionally, it's more explicit and cleaner to conditionally add the property only when it's defined. Consider using: ...(assetsPrefix ? { assetsPrefix } : {}) instead of assetsPrefix: assetsPrefix to avoid adding undefined properties to the config objects.

Suggested change
// Add the assetsPrefix passed to the component to each font config
assetsPrefix: assetsPrefix,
// Add the assetsPrefix passed to the component to each font config, only when defined
...(assetsPrefix ? { assetsPrefix } : {}),

Copilot uses AI. Check for mistakes.
}));

const resolvedConfig = await generateFonts(configWithPrefix)

const preloads = resolvedConfig.map(createPreloads)
const styles = Promise.all([...resolvedConfig.map(createBaseCSS), ...resolvedConfig.map(createFontCSS)])
Expand Down
45 changes: 33 additions & 12 deletions packages/astro-font/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ interface Config {
fallback: 'serif' | 'sans-serif' | 'monospace'
// https://developer.mozilla.org/en-US/docs/Web/CSS/@font-face/font-display
display: 'auto' | 'block' | 'swap' | 'fallback' | 'optional' | (string & {})
assetsPrefix?: string
}

export interface Props {
Expand All @@ -72,7 +73,25 @@ const extToPreload = {
}

function getBasePath(src?: string) {
return src || './public'
return src || './public';
}

export function generatePublicUrl(localPath: string, basePath: string, assetsPrefix?: string): string {
if (localPath.startsWith('https:') || localPath.startsWith('http:')) {
return localPath;
}
Comment on lines +80 to +82

Copilot AI Feb 17, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The absolute URL detection only checks for https: and http: protocols, but doesn't handle protocol-relative URLs (starting with //) or other schemes like file://. While these cases may be rare in practice, consider using a more comprehensive check such as testing if the path contains :// or starts with // to handle all absolute URL formats consistently. For example: if (localPath.includes('://') || localPath.startsWith('//')) would be more robust.

Copilot uses AI. Check for mistakes.
// Calculate relative path using the logic similar to the original getRelativePath
const pathRelativeToPublic = relative(basePath, localPath);

if (assetsPrefix) {
// Ensure prefix ends with a slash and relative path doesn't start with one
const normalizedPrefix = assetsPrefix.endsWith('/') ? assetsPrefix : assetsPrefix + '/';
const normalizedRelative = pathRelativeToPublic.startsWith('/') ? pathRelativeToPublic.substring(1) : pathRelativeToPublic;
return normalizedPrefix + normalizedRelative;
Comment on lines +86 to +90

Copilot AI Feb 17, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The generatePublicUrl function should validate the assetsPrefix parameter to prevent potential security issues. If a user provides a malicious prefix containing special characters or path traversal sequences (e.g., javascript:, data:, or ../), this could lead to XSS vulnerabilities or unintended behavior. Consider adding validation to ensure the assetsPrefix starts with http://, https://, or /, and doesn't contain dangerous protocols or path traversal patterns.

Copilot uses AI. Check for mistakes.
} else {
// Original behavior: return root-relative path, ensuring it starts with '/'
return '/' + (pathRelativeToPublic.startsWith('/') ? pathRelativeToPublic.substring(1) : pathRelativeToPublic);
Comment on lines +85 to +93

Copilot AI Feb 17, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The path normalization logic could produce incorrect results on Windows when pathRelativeToPublic contains backslashes. The relative() function from pathe should handle cross-platform paths, but the string manipulation with startsWith('/') and substring(1) assumes forward slashes. Consider normalizing the path separators before the slash checks, or use pathe's normalize() function to ensure consistent forward slashes across platforms. For example: const normalizedRelative = pathRelativeToPublic.replace(/\\/g, '/').replace(/^\//, '');

Suggested change
if (assetsPrefix) {
// Ensure prefix ends with a slash and relative path doesn't start with one
const normalizedPrefix = assetsPrefix.endsWith('/') ? assetsPrefix : assetsPrefix + '/';
const normalizedRelative = pathRelativeToPublic.startsWith('/') ? pathRelativeToPublic.substring(1) : pathRelativeToPublic;
return normalizedPrefix + normalizedRelative;
} else {
// Original behavior: return root-relative path, ensuring it starts with '/'
return '/' + (pathRelativeToPublic.startsWith('/') ? pathRelativeToPublic.substring(1) : pathRelativeToPublic);
// Normalize path separators and remove any leading slash for consistent URL construction
const normalizedRelative = pathRelativeToPublic.replace(/\\/g, '/').replace(/^\//, '');
if (assetsPrefix) {
// Ensure prefix ends with a slash and relative path doesn't start with one
const normalizedPrefix = assetsPrefix.endsWith('/') ? assetsPrefix : assetsPrefix + '/';
return normalizedPrefix + normalizedRelative;
} else {
// Original behavior: return root-relative path, ensuring it starts with '/'
return '/' + normalizedRelative;

Copilot uses AI. Check for mistakes.
}
}

export function getRelativePath(from: string, to: string) {
Expand Down Expand Up @@ -330,27 +349,29 @@ async function getFallbackFont(fontCollection: Config): Promise<Record<string, s
}

export function createPreloads(fontCollection: Config): string[] {
// If the parent preload is set to be false, look for true only preload values
if (fontCollection.preload === false) {
return fontCollection.src
.filter((i) => i.preload === true)
.map((i) => getRelativePath(getBasePath(fontCollection.basePath), i.path))
}
// If the parent preload is set to be true (or not defined), look for non-false values
return fontCollection.src
.filter((i) => i.preload !== false)
.map((i) => getRelativePath(getBasePath(fontCollection.basePath), i.path))
const effectiveBasePath = getBasePath(fontCollection.basePath);
const sourcesToPreload = fontCollection.preload === false
? fontCollection.src.filter((i) => i.preload === true)
: fontCollection.src.filter((i) => i.preload !== false);

// Generate public URLs using the new function
return sourcesToPreload.map((i) =>
generatePublicUrl(i.path, effectiveBasePath, fontCollection.assetsPrefix)
);
}

export async function createBaseCSS(fontCollection: Config): Promise<string[]> {
try {
const effectiveBasePath = getBasePath(fontCollection.basePath);
Comment thread
rishi-raj-jain marked this conversation as resolved.
const tmp = fontCollection.src.map((i) => {
const cssProperties = Object.entries(i.css || {}).map(([key, value]) => `${key}: ${value}`)
if (i.weight) cssProperties.push(`font-weight: ${i.weight}`)
if (i.style) cssProperties.push(`font-style: ${i.style}`)
if (fontCollection.name) cssProperties.push(`font-family: '${fontCollection.name}'`)
if (fontCollection.display) cssProperties.push(`font-display: ${fontCollection.display}`)
cssProperties.push(`src: url(${getRelativePath(getBasePath(fontCollection.basePath), i.path)})`)

const publicUrl = generatePublicUrl(i.path, effectiveBasePath, fontCollection.assetsPrefix);
cssProperties.push(`src: url(${publicUrl})`);
return `@font-face {${cssProperties.join(';')}}`
})
return tmp
Expand Down