Closed lojjic closed 1 year ago
Thanks for the PR! This is an interesting but breaking change in terms of browser support.
Is the browser support loss major? Not really. I do like the fact it unifies the behaviour of APIv2 but I need to do a much deeper review of this and its effects when I'm actively working on the Fontsource v5 CLI (possibly next month) since this is a breaking change all around. Looks good so far!
As for fixtures/google-fonts-v2.json
. That's a manual change to update. Honestly better off as a snapshot 😅
If it would avoid breaking downstream Fontsource code, this could also be approached by adding it as an additional format, something like "woff-split"
, rather than overwriting the "woff"
. For my use case I'm more interested in using this metadata directly rather than the Fontsource packages, so it would serve my needs that way. Food for thought. 🤷♂️
If we can get tests passing, I don't think there's any issue merging this and releasing a new major version. I think this is a solid intuitive change to make.
The APIv2 fixture is a manually updated fixture but feel free to change that into a snapshot to clean things up in the future.
Downstream won't have any issues since they'll just rely on an older version until the dep is updated.
Once again, thank you for the PR! I've updated the tests for it to work in this case.
An added benefit of this is reducing package sizes for Fontsource! Noto Serif HK was around 250MB which just about trips NPM package size limitations. This PR reduces that package to around 190MB instead. 🎉
An added benefit of this is reducing package sizes for Fontsource!
Oh! That's actually the opposite of what I'd have expected, I was under the impression that the sum of individual files would be larger than the combined file due to repetition of common data across all of them. 🤔
Oh! That's actually the opposite of what I'd have expected, I was under the impression that the sum of individual files would be larger than the combined file due to repetition of common data across all of them. 🤔
Just a guess, but Google's subsetting probably cuts out a bunch of glyphs. I've seen people mention that the all
woff files supports more characters and features than woff2 variants, so that may hold some bearing.
https://github.com/fontsource/fontsource/issues/643 - Adding this for future reference on why these files are smaller.
This removes the prior limitation for
woff
files in the v2 API where it would use a single combined file for all unicode-range subsets. Safari 11 briefly had support forunicode-range
but didn't yet supportwoff2
, so if you use a Safari 11 user-agent then GFonts will serve up CSS with individualwoff
files per subset, just likewoff2
.I've regenerated the data files locally with this change and verified the output includes the subset woff files. Sample diff:
I was unable to get the unit tests passing with this change -- I think
fixtures/google-fonts-v2.json
needs to be regenerated but I was unable to figure out how, it doesn't get updated by the generate-fixtures script. I'm happy to do so with some instruction.Thanks!