Skip to content

fix(fonts): do not filter sources - #13646

Merged
florian-lefebvre merged 1 commit into
mainfrom
fix/fonts-preloads-filter
Apr 17, 2025
Merged

florian-lefebvre merged 1 commit into
mainfrom
fix/fonts-preloads-filter

Conversation

@florian-lefebvre

Copy link
Copy Markdown
Member

Changes

  • In feat(fonts): unifont improvements #13594 in order to avoid too many preloads, I filtered the font sources so there are less
  • However, this was not the right fix. Instead we want to preserve all sources but only add the first one to be preloaded

Testing

Manual

Docs

Changeset

@florian-lefebvre florian-lefebvre self-assigned this Apr 17, 2025
@changeset-bot

changeset-bot Bot commented Apr 17, 2025

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: dbd769d

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions Bot added the pkg: astro Related to the core `astro` package (scope) label Apr 17, 2025
@codspeed

codspeed Bot commented Apr 17, 2025

Copy link
Copy Markdown

CodSpeed Performance Report

Merging #13646 will not alter performance

Comparing fix/fonts-preloads-filter (dbd769d) with main (23410c6)

Summary

✅ 6 untouched benchmarks

@ematipico ematipico left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Left a nit

Comment on lines +130 to +151
...font,
src: font.src.map((source) => {
if ('name' in source) {
return source;
}
const proxied = {
...source,
originalURL: source.url,
url: proxyURL({
value: source.url,
// We only use the url for hashing since the service returns urls with a hash already
hashString,
// We only collect the first URL to avoid preloading fallback sources (eg. we only
// preload woff2 if woff is available)
collect: (data) => collect(data, index === 0),
}),
};
index++;
return proxied;
}),
};
});

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: for readability, I would put this code before the return, and maybe use a for loop instead

@florian-lefebvre
florian-lefebvre merged commit 6744842 into main Apr 17, 2025
@florian-lefebvre
florian-lefebvre deleted the fix/fonts-preloads-filter branch April 17, 2025 14:11
@astrobot-houston astrobot-houston mentioned this pull request Apr 17, 2025
openscript pushed a commit to openscript/astro that referenced this pull request Sep 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pkg: astro Related to the core `astro` package (scope)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants