vitejs / vite

Next generation frontend tooling. It's fast!
http://vite.dev
MIT License
68.5k stars 6.18k forks source link

Variable-width fonts not resolving during Vite build #15109

Closed ndv99 closed 11 months ago

ndv99 commented 11 months ago

Describe the bug

I'm currently converting a large web app from create-react-app to Vite. Our SCSS framework uses variable width & weight fonts, which are imported using url() in SCSS. We need to provide these fonts as static assets in our project, since our users often use it in environments without an internet connection to fetch webfonts with.

The font file names contain square brackets [ for the weight and width to be injected into. However, using square brackets in the import string results in incorrect font file names in the build output, where the square brackets are replaced with underscores. So, our framework replaces the brackets with percent-encoded characters - %5B for [, and %5D for ]. However, Vite/Rollup does not decode these while building, and the font imports do not resolve.

Reproduction

https://stackblitz.com/edit/vitejs-vite-tsnz33?file=style.css

Steps to reproduce

Using the provided link:

  1. Run npm run build
  2. Observe error about a .woff2 file not resolving at build time
  3. In style.css, comment out line 6 and uncomment line 7
  4. Run npm run build
  5. Observe font file being produced, where underscores replace the square brackets

System Info

System:
    OS: Linux 6.5 Ubuntu 23.10 23.10 (Mantic Minotaur)
    CPU: (24) x64 AMD Ryzen 9 3900X 12-Core Processor
    Memory: 17.39 GB / 31.26 GB
    Container: Yes
    Shell: 5.2.15 - /bin/bash
  Binaries:
    Node: 20.9.0 - ~/.nvm/versions/node/v20.9.0/bin/node
    Yarn: 1.22.21 - ~/.nvm/versions/node/v20.9.0/bin/yarn
    npm: 10.1.0 - ~/.nvm/versions/node/v20.9.0/bin/npm
  Browsers:
    Chrome: 119.0.6045.105
  npmPackages:
    @vitejs/plugin-react-swc: 3.4.1 => 3.4.1 
    vite: 4.5.0 => 4.5.0

Used Package Manager

yarn

Logs

Unresolved font file output ```shell ❯ npm run build --debug > vite-starter@0.0.0 build > vite build vite v5.0.2 building for production... transforming (1) index.html /fonts/f1ea362b-Ubuntu%5Bwdth,wght%5D-latin-v0.896a.woff2 referenced in /home/projects/vitejs-vite-tsnz33/style.css didn't resolve at build time, it will remain unchanged to be resolved at runtime ✓ 7 modules transformed. dist/index.html 0.45 kB │ gzip: 0.30 kB dist/assets/index-GZD-3WAI.css 1.36 kB │ gzip: 0.70 kB dist/assets/index-5_-lRlrZ.js 2.59 kB │ gzip: 1.38 kB ✓ built in 783ms ```
Resolved font with incorrect name output ```shell ❯ npm run build --debug > vite-starter@0.0.0 build > vite build vite v5.0.2 building for production... ✓ 7 modules transformed. dist/index.html 0.45 kB │ gzip: 0.29 kB dist/assets/f1ea362b-Ubuntu_wdth_wght_-latin-v0.896a-b_Xgw9oJ.woff2 95.13 kB dist/assets/index-CccUqCrK.css 1.37 kB │ gzip: 0.70 kB dist/assets/index-UuEVhJ86.js 2.59 kB │ gzip: 1.38 kB ✓ built in 759ms ```

Validations

summer-boythink commented 11 months ago

https://github.com/vitejs/vite/blob/main/packages/vite/src/node/plugins/css.ts#L1395

Seems to have something to do with this line of code?

Because here only match the regular expression, and did not do decodeURIComponent? This is just my guess, I hope someone to discuss

dejour commented 11 months ago

square bracket [ transforming to _ in the output filename is expected behavior as per the source code of rollup. but font url in css file and output filename are same. so it would loads correctly. do you need to preserve that square bracket?

const INVALID_CHAR_REGEX = /[\u0000-\u001F"#$&*+,:;<=>?[\]^`{|}\u007F]/g;
const DRIVE_LETTER_REGEX = /^[a-z]:/i;
function sanitizeFileName(name) {
    const match = DRIVE_LETTER_REGEX.exec(name);
    const driveLetter = match ? match[0] : '';
    // A `:` is only allowed as part of a windows drive letter (ex: C:\foo)
    // Otherwise, avoid them because they can refer to NTFS alternate data streams.
    return driveLetter + name.slice(driveLetter.length).replace(INVALID_CHAR_REGEX, '_');
}
sapphi-red commented 11 months ago

@summer-boythink Yes, to make %5B work, I think we need to call decodeURI at https://github.com/vitejs/vite/blob/0654d1b52448db4d7a9b69aee6aad9e015481452/packages/vite/src/node/plugins/css.ts#L266-L276

But even will that Vite will still output the file name with _ instead of [ as @dejour described. You can set build.rollupOptions.output.sanitizeName to allow [. But if you want to preserve the file name, I'd suggest using the public directory instead.

summer-boythink commented 11 months ago

@summer-boythink Yes, to make %5B work, I think we need to call decodeURI at

https://github.com/vitejs/vite/blob/0654d1b52448db4d7a9b69aee6aad9e015481452/packages/vite/src/node/plugins/css.ts#L266-L276

But even will that Vite will still output the file name with _ instead of [ as @dejour described. You can set build.rollupOptions.output.sanitizeName to allow [. But if you want to preserve the file name, I'd suggest using the public directory instead.

Thank you for your answer

ndv99 commented 11 months ago

square bracket [ transforming to _ in the output filename is expected behavior as per the source code of rollup. but font url in css file and output filename are same. so it would loads correctly. do you need to preserve that square bracket?

const INVALID_CHAR_REGEX = /[\u0000-\u001F"#$&*+,:;<=>?[\]^`{|}\u007F]/g;
const DRIVE_LETTER_REGEX = /^[a-z]:/i;
function sanitizeFileName(name) {
    const match = DRIVE_LETTER_REGEX.exec(name);
    const driveLetter = match ? match[0] : '';
    // A `:` is only allowed as part of a windows drive letter (ex: C:\foo)
    // Otherwise, avoid them because they can refer to NTFS alternate data streams.
    return driveLetter + name.slice(driveLetter.length).replace(INVALID_CHAR_REGEX, '_');
}

I do need to preserve the square bracket. The font is imported within our SCSS framework library, so it's not something I can change easily unfortunately, and the browser also decodes the font URI so the final file is expected to have a square bracket.

ndv99 commented 11 months ago

@summer-boythink Yes, to make %5B work, I think we need to call decodeURI at

https://github.com/vitejs/vite/blob/0654d1b52448db4d7a9b69aee6aad9e015481452/packages/vite/src/node/plugins/css.ts#L266-L276

But even will that Vite will still output the file name with _ instead of [ as @dejour described. You can set build.rollupOptions.output.sanitizeName to allow [. But if you want to preserve the file name, I'd suggest using the public directory instead.

So I tried setting build.rollupOptions.output.sanitizeName to this:

(name: string) => decodeURI(name)

but the font files still don't resolve at runtime, and the error message is the same, showing the encoded characters:

/assets/fonts/f1ea362b-Ubuntu%5Bwdth,wght%5D-latin-v0.896a.woff2 referenced in /home/my-username/Repositories/maas-ui/src/scss/index.scss didn't resolve at build time, it will remain unchanged to be resolved at runtime

I've updated the minimal reproduction to replicate this.