search

storybookjs / storybook

Storybook is the industry standard workshop for building, documenting, and testing UI components in isolation
https://storybook.js.org
MIT License
84.59k stars 9.31k forks source link

Local font preloading broken in Storybook 6.2.x with webpack 5 builder #14644

Closed mnquintana closed 1 year ago

mnquintana commented 3 years ago

Describe the bug

It seems that preloading fonts from a local static directory in .storybook/preview-head.html is broken as of Storybook 6.2. This is reproducible both with the ~default Webpack 4 and~ new Webpack 5 builder. It's not reproducible in Storybook 6.1.x, or on Storybook 6.2.x with Webpack 4.

Example preview-head.html

For a file located at /public/Inter.woff2, and with Storybook run with ./public as a --static-dir:

<link
  rel="preload"
  href="/Inter.woff2"
  as="font"
  type="font/woff2"
  crossorigin
/>

There's a workaround for the bug in development builds (change the URL to start with /static/media/public/), but production builds additionally don't work because Storybook inserts a hash in the filename now (see repro steps below).

Let me know if there's any other info I could provide that'd be helpful for debugging!

To Reproduce

Minimal Repro Repo

Prerequisites

  1. Make sure you have a webfont in a local directory in your project (e.g. /public/<font>.woff2)
  2. Make you're passing in the path to the static directory to both storybook executables (e.g. start-storybook --static-dir ./public and build-storybook --static-dir ./public)
  3. Make sure you have a .storybook/preview-head.html and it includes this <link>:
<link
  rel="preload"
  href="/<font>.woff2"
  as="font"
  type="font/woff2"
  crossorigin
/>
  1. Make sure you have a CSS file that references the font file by relative path on desk (e.g. /styles/global.css):
@font-face {
  font-family: "Custom";
  src: url("../public/Inter.woff2") format("woff2");
}
  1. Make sure you have a component that uses the custom family in its styles
  2. Make sure you're using @storybook/builder-webpack5 and have { core: { builder: 'webpack5' } } set in your .storybook/main.js

Production

  1. Run build-storybook --static-dir ./public
  2. Launch the production build of storybook
  3. Open devtools once Storybook launches, and reload the page
  4. Filter by Fonts in the Network panel

Expected

<font>.woff2 is successfully preloaded from /<font>.woff2.

Actual

System

Please paste the results of npx sb@next info here.

Environment Info:

  System:
    OS: macOS 10.15.7
    CPU: (16) x64 Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
  Binaries:
    Node: 15.11.0 - ~/.nvm/versions/node/v15.11.0/bin/node
    Yarn: 1.22.10 - /usr/local/bin/yarn
    npm: 7.10.0 - ~/.nvm/versions/node/v15.11.0/bin/npm
  Browsers:
    Chrome: 89.0.4389.128
    Firefox: 86.0
    Safari: 14.0.3
  npmPackages:
    @storybook/addon-actions: ^6.2.8 => 6.2.8 
    @storybook/addon-essentials: ^6.2.8 => 6.2.8 
    @storybook/addon-links: ^6.2.8 => 6.2.8 
    @storybook/builder-webpack5: ^6.2.8 => 6.2.8 
    @storybook/react: ^6.2.8 => 6.2.8

┆Issue is synchronized with this Asana task by Unito

shilman commented 3 years ago

Was this working as expected in a previous version of Storybook?

mnquintana commented 3 years ago

It was! It was working as expected in 6.1.21.

shilman commented 3 years ago

Do you have a repro repo you can share?

mnquintana commented 3 years ago

Sure!

From some additional testing, it seems like this is only broken with the Webpack 5 builder after all. This is the culprit: https://github.com/mnquintana/storybook-font-preload-repro/blob/storybook%406.2-webpack5-minimal-repro/styles/global.css#L4-L6

Something changed with how paths are resolved in CSS files in the Webpack 5 builder. When I tried using the original paths from Storybook 6.1, Webpack threw this error:

ModuleBuildError: Module build failed (from ./node_modules/@storybook/builder-webpack5/node_modules/css-loader/dist/cjs.js):
Error: Can't resolve '/assets/fonts/Inter.woff2' in '/Users/mnquintana/code/storybook-font-preload-repro/styles'
    at finishWithoutResolve (/Users/mnquintana/code/storybook-font-preload-repro/node_modules/@storybook/builder-webpack5/node_modules/enhanced-resolve/lib/Resolver.js:293:18)
    at /Users/mnquintana/code/storybook-font-preload-repro/node_modules/@storybook/builder-webpack5/node_modules/enhanced-resolve/lib/Resolver.js:362:15
    at /Users/mnquintana/code/storybook-font-preload-repro/node_modules/@storybook/builder-webpack5/node_modules/enhanced-resolve/lib/Resolver.js:410:5
    at eval (eval at create (/Users/mnquintana/code/storybook-font-preload-repro/node_modules/@storybook/builder-webpack5/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:16:1)
    at /Users/mnquintana/code/storybook-font-preload-repro/node_modules/@storybook/builder-webpack5/node_modules/enhanced-resolve/lib/Resolver.js:410:5
    at eval (eval at create (/Users/mnquintana/code/storybook-font-preload-repro/node_modules/@storybook/builder-webpack5/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:27:1)
    at /Users/mnquintana/code/storybook-font-preload-repro/node_modules/@storybook/builder-webpack5/node_modules/enhanced-resolve/lib/DescriptionFilePlugin.js:87:43
    at /Users/mnquintana/code/storybook-font-preload-repro/node_modules/@storybook/builder-webpack5/node_modules/enhanced-resolve/lib/Resolver.js:410:5
    at eval (eval at create (/Users/mnquintana/code/storybook-font-preload-repro/node_modules/@storybook/builder-webpack5/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:16:1)
    at /Users/mnquintana/code/storybook-font-preload-repro/node_modules/@storybook/builder-webpack5/node_modules/enhanced-resolve/lib/forEachBail.js:16:12
    at /Users/mnquintana/code/storybook-font-preload-repro/node_modules/@storybook/builder-webpack5/node_modules/enhanced-resolve/lib/AliasPlugin.js:103:14
    at next (/Users/mnquintana/code/storybook-font-preload-repro/node_modules/@storybook/builder-webpack5/node_modules/enhanced-resolve/lib/forEachBail.js:14:3)
    at forEachBail (/Users/mnquintana/code/storybook-font-preload-repro/node_modules/@storybook/builder-webpack5/node_modules/enhanced-resolve/lib/forEachBail.js:24:9)
    at /Users/mnquintana/code/storybook-font-preload-repro/node_modules/@storybook/builder-webpack5/node_modules/enhanced-resolve/lib/AliasPlugin.js:37:5
    at _next0 (eval at create (/Users/mnquintana/code/storybook-font-preload-repro/node_modules/@storybook/builder-webpack5/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:8:1)
    at eval (eval at create (/Users/mnquintana/code/storybook-font-preload-repro/node_modules/@storybook/builder-webpack5/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:30:1)
    at processResult (/Users/mnquintana/code/storybook-font-preload-repro/node_modules/@storybook/builder-webpack5/node_modules/webpack/lib/NormalModule.js:676:19)
    at /Users/mnquintana/code/storybook-font-preload-repro/node_modules/@storybook/builder-webpack5/node_modules/webpack/lib/NormalModule.js:778:5
    at /Users/mnquintana/code/storybook-font-preload-repro/node_modules/@storybook/builder-webpack5/node_modules/loader-runner/lib/LoaderRunner.js:399:11
    at /Users/mnquintana/code/storybook-font-preload-repro/node_modules/@storybook/builder-webpack5/node_modules/loader-runner/lib/LoaderRunner.js:251:18
    at context.callback (/Users/mnquintana/code/storybook-font-preload-repro/node_modules/@storybook/builder-webpack5/node_modules/loader-runner/lib/LoaderRunner.js:124:13)
    at Object.loader (/Users/mnquintana/code/storybook-font-preload-repro/node_modules/@storybook/builder-webpack5/node_modules/css-loader/dist/index.js:154:5)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (node:internal/process/task_queues:94:5)

That same path resolves as expected with the Webpack 4 builder on Storybook 6.2.8.

I had to change that path in global.css to the relative path on disk for Webpack to resolve the path properly, but then it broke font preloading in production builds because Webpack 5 resolves that path to one that includes a hash in the filename, which can't be referenced from within preview-head.html.

shilman commented 3 years ago

@mnquintana so it works with webpack4 in 6.2?

mnquintana commented 3 years ago

@mnquintana so it works with webpack4 in 6.2?

It does! Let me know if there's any more info I could provide that'd be helpful for debugging.

shilman commented 3 years ago

Thanks so much for all the info, this is wonderful 🙏

nvegater commented 3 years ago

found any workaround in the meantime that doesn't involve changing versions ? I'm using 6.3.8 in storybook react.

sal-lochbaum commented 3 years ago

I had the same issue.

My Fonts are in src/fonts/ and are included in my stylesheet via scss imports, thus emitted by webpack as an asset. For preloading to work the font definition in the css must use the same url as the tag in the head – using storybooks static file serving isn't an option in that case.

The default webpack configuration used by storybook generates different filenames depending on the production flag (which is probably when you run storybook dynamically and true when you build it). https://github.com/storybookjs/storybook/blob/64122cb5201b5a0473fdc079726610cf39c04ad4/lib/builder-webpack5/src/preview/base-webpack.config.ts#L54-L62

So if you add one url to your preview-head.html it will only work in one mode and not the other.

The workaround is to simply preload both urls and live with the 404 error

e.g.

<!-- Font Preloading in dynamic mode -->
<link rel="preload" as="font" type="font/woff2" href="/static/media/src/fonts/5262913/8a7eda06-48ab-4713-bddd-338da4dc23bd..woff2" crossorigin>
<!-- Font Preloading in built -->
<link rel="preload" as="font" type="font/woff2" href="/static/media/8a7eda06-48ab-4713-bddd-338da4dc23bd.a52b710f..woff2" crossorigin>

Alternative workaround:

Use the webpackFinal option and change the rule mentioned above to use the same filename in both modes.

For a proper fix we would need access to the generated filename in preview-head.html or in the previewHead option.

Or integrate a webpack plugin like https://www.npmjs.com/package/webpack-font-preload-plugin to change the header?

shilman commented 1 year ago

We’re cleaning house! Storybook has changed a lot since this issue was created and we don’t know if it’s still valid. Please open a new issue referencing this one if: