Google Fonts with weights above 900 won't load #54304
Comments
github-actions
bot
added
the
Font (next/font)
|
It seems like in the case of the URL generated by Explanation: Consider an array named "numbers" with the values (weights of fonts) assigned to it.
When the sort function is called on it without passing any comparison function, it gives us the result:
which is "not" the desired outcome. The reason is, due to the default behaviour of the Array.prototype.sort() method in JavaScript. When no comparison function is provided, the sort() method converts the elements of the array to strings and then performs a lexicographic (dictionary-style) sorting. In this case, all the elements in the "numbers" array are strings, so they are sorted lexicographically. When lexicographically sorted, "1000" comes before "200" because "1" comes before "2" in the character encoding. By providing the comparison function (a, b) => parseInt(a) - parseInt(b), it will ensure that the elements are compared as integers, and thus the sorting will be done numerically. I think we should check where this sorting is taking place. |
|
@indraantoor thanks for the great explanation, I had thought something similar as well. |
## What? This PR fixes issue #54304. As discussed in the issue it was suspected that the values of weights were not being sorted correctly. The weights were being sorted in lexicographical order. In order for Google fonts to work, Google Fonts need the values of weights present in the url in sorted order. Due to this the url which was being generated for Google Fonts which had a value of "1000" in weight was giving an error and the font was not working. ## Why I Think This Solution Works? Let's assume that we have an object named axes with the property named named "wght" which will have the value of weights as following: ``` const axes = { wght: ["100", "200", "300", "400", "500", "600", "700", "800", "900", '1000'] } ``` Let's create another variable named "display" with the value set as "swap" and create another variable named "fontFamily" with the value as "DM Sans" (this is the font which was being used in the issue). ``` const fontFamily = "DM Sans"; const display = "swap"; ``` If we call the function using them ``` const FONT_URL = getGoogleFontsUrl(fontFamily, axes, display); console.log("The font url is: ", FONT_URL) Output Will Be: The font url is: https://fonts.googleapis.com/css2?family=DM+Sans:wght@100;1000;200;300;400;500;600;700;800;900&display=swap ``` It will give the incorrect value which was shown in the issue. If you see at line number 56 there was no comparison function passed in the sort function which is what was suspected earlier and it was sorting the values lexicographically. Create another variable inside the function above line 54 where we are updating the value of "url" variable. ``` const variantValues = variants.map((variant) => variant.map(([, val]) => val).join(',')) console.log("Variant Values: ", variantValues) Output Will Be: Variant Values: ["100", "200", "300", "400", "500", "600", "700", "800", "900", "1000"] ``` Now call the function again with the same values you will see the values in the correct order but as soon as you call "sort" function it will give you incorrect sorted values which is what is happening in the "url" variable. ``` const variantValues = variants.map((variant) => variant.map(([, val]) => val).join(',')).sort() console.log("Variant Values: ", variantValues) Output Will Be: ["100", "1000", "200", "300", "400", "500", "600", "700", "800", "900"] (As you can see the values are sorted incorrectly) ``` There was one case when I implemented this solution. When "ital" is also passed then the values are in "ital,wght" format and because of it the sorting was giving incorrect result for this case. Let's update the "axes" variable which we made previously to contain these values: ``` const axes = { wght: ["500","300", "400"], ital: ["0","1"] } ``` Now when we run the function it gives us the result as: ` https://fonts.googleapis.com/css2?family=DM+Sans:ital,wght@0,500;0,300;0,400;1,500;1,300;1,400&display=swap` Where as the expected result was: `https://fonts.googleapis.com/css2?family=DM+Sans:ital,wght@0,300;0,400;0,500;1,300;1,400;1,500&display=swap` So, I rewrote the "sort" function to also handle this case. ## What Did I Do To Fix It? I passed the comparison function inside the "sort" function which was being to ensure that the elements are compared as integers, and thus the sorting will be done numerically. Which gave me the correct output as url: `https://fonts.googleapis.com/css2?family=DM+Sans:wght@100;200;300;400;500;600;700;800;900;1000&display=swap` With values being sorted correctly and if you visit the link you will see the response instead of the error. Any thought's or suggestions would be greatly appreciated.
|
This closed issue has been automatically locked because it had no new activity for 2 weeks. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you. |
Verify canary release
Provide environment information
Operating System: Platform: linux Arch: x64 Version: #1 SMP PREEMPT_DYNAMIC Thu Jul 27 19:56:37 UTC 2023 Binaries: Node: 18.16.0 npm: 9.5.1 Yarn: 1.22.19 pnpm: 8.6.8 Relevant Packages: next: 13.4.20-canary.0 eslint-config-next: 13.4.2 react: 18.2.0 react-dom: 18.2.0 typescript: 5.1.6 Next.js Config: output: N/AWhich area(s) of Next.js are affected? (leave empty if unsure)
Font optimization (next/font)
Link to the code that reproduces this issue or a replay of the bug
https://codesandbox.io/p/sandbox/strange-matsumoto-nlnw4p?file=/app/layout.tsx:32,41
To Reproduce
Describe the Bug
Can't load DM sans even though it's not a network issue. This is likely due to the generated Google Fonts URL for DM Sans being rejected, if the font weights aren't sorted in the correct order (as shown below):
next/font(1000 comes after 100):Expected Behavior
DM sans font should be able to load with all weights selected
Which browser are you using? (if relevant)
Brave Version 1.56.20 Chromium: 115.0.5790.171
How are you deploying your application? (if relevant)
locally for now, but I'd assume this would be the same on vercel
The text was updated successfully, but these errors were encountered: