vercel / next.js

The React Framework
https://nextjs.org
MIT License
125.46k stars 26.8k forks source link

The font loader should tolerate backtick strings without template variables #63041

Open arcanis opened 6 months ago

arcanis commented 6 months ago

Link to the code that reproduces this issue

https://codesandbox.io/p/devbox/modest-mestorf-42dh9l

To Reproduce

  1. Add a next/font call with backtick strings:
import {Inter} from 'next/font/google';

const inter = Inter({
  subsets: [`latin`],
  display: `swap`,
  variable: `--font-inter`,
});
  1. Run the application

Current vs. Expected behavior

Next.js crashes on template-less backtick strings when used within a localFont call. It should tolerate such strings, as they are fully static. Some lint rules even enforce their use everywhere possible, rather than ' or ".

Provide environment information

Operating System:
  Platform: darwin
  Arch: arm64
  Version: Darwin Kernel Version 23.3.0: Wed Dec 20 21:30:44 PST 2023; root:xnu-10002.81.5~7/RELEASE_ARM64_T6000
Binaries:
  Node: 20.11.1
  npm: 10.2.4
  Yarn: 4.1.1
  pnpm: N/A
Relevant Packages:
  next: 14.1.3
  eslint-config-next: 14.1.3
  react: 18.2.0
  react-dom: 18.2.0
  typescript: 5.4.2
Next.js Config:
  output: N/A

Which area(s) are affected? (Select all that apply)

Font optimization (next/font)

Which stage(s) are affected? (Select all that apply)

next dev (local), next build (local)

Additional context

No response

mutasim77 commented 6 months ago

This one is not a bug. The error message is clear: Font loader values must be explicitly written literals, suggesting that the code you're using to load fonts expects specific literal values and doesn't accept dynamic or computed values. When you're using template literals (backticks) like this:

const inter = Inter({
  subsets: [`latin`],
});

The function Inter expects literal values without any computation or interpolation. Even though 'latin' is a string literal, using it within backticks causes the font loader to interpret it differently.

If you're curious and want to know how and why, here's the source code written in Rust: code


Using backticks is preferred when you need to interpolate variables or expressions or when you want to format a string block. Even if you look at JS style guides, JS coding standards, and so on, most of them recommend using single quotes for regular and simple strings.

arcanis commented 6 months ago

I understand the error message; the point I'm making here is that a template literal that doesn't contain interpolated expressions has no dependencies on the runtime, and is thus a static string literal.

In other words, what you're saying here is exactly the bug I'm referencing:

using it within backticks causes the font loader to interpret it differently.

Using it within backticks should not cause the font loader to interpret it differently (as long as it doesn't contain any substitution), simply because there's no reasons to do so. Backtick strings are the most refactoring-friendly ones, and while I agree some stylistic guides favour single quotes, it so happens that others favour backticks.

As an example, TypeScript permits backtick strings in places that only permit literals, as long as they don't contain any substitutions. They even have their own AST node: NoSubstitutionTemplateLiteral.

mkvlrn commented 3 months ago

Finally tweaking my brand new eslint flat config to just force backticks everywhere brought me here! Across all my projects where the config is now applied only next projects where next/font/google is used raised any problems.

I suppose it's easy enough to tweak the configuration further or just disable the rule on those lines, but here's hoping @arcanis ' point is enough to enact an investigation.