-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
NextJS: Support @next/font #20291
NextJS: Support @next/font #20291
Conversation
5cc1898
to
4383d37
Compare
49b507a
to
0384b99
Compare
ca793db
to
43b371b
Compare
43b371b
to
17f6567
Compare
874f015
to
b782c95
Compare
Anything in the way from blocking this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EDIT: turns out the issue is related to the export
keyword. The AST transformation was not applied to ExportNamedDeclarations
, probably not for default either
@valentinpalkovic it seems like if the fonts are imported in the .stories file, it's all good, but if they come from a separate file, it breaks:
Examples of separate file:
Button used in Button.stories.js:
import React from 'react';
import PropTypes from 'prop-types';
import './button.css';
import { Rubik } from '@next/font/google';
export const rubik = Rubik({ subsets: ['latin'], variable: '--font-latin-rubik', weight: '400' });
/**
* Primary UI component for user interaction
*/
export const Button = ({ primary, backgroundColor, size, label, ...props }) => {
const mode = primary ? 'storybook-button--primary' : 'storybook-button--secondary';
return (
<button
type="button"
className={[rubik.className, 'storybook-button', `storybook-button--${size}`, mode].join(' ')}
{...props}
>
{label}
<style jsx>{`
button {
background-color: ${backgroundColor};
}
`}</style>
</button>
);
};
a separate fonts.js
file (I saw this documented somewhere, but don't remember):
import { Fira_Code } from '@next/font/google';
export const fira = Fira_Code({ subsets: ['latin']})
@yannbf Thank you! Indeed, if the Variable declaration was a import localFont from '@next/font/local';
// The export case was not considered in the babel transformation
export const localRubikStorm = localFont({
src: '/fonts/RubikStorm-Regular.ttf',
variable: '--font-rubik-storm',
}); This should now be fixed in the latest commit. Now the above code will be transformed correctly to: import localRubikStorm from "@next/font/local?localFont;{\"src\":\"/fonts/RubikStorm-Regular.ttf\",\"variable\":\"--font-rubik-storm\"}"; // The export case was not considered in the babel transformation
export { localRubikStorm }; Please delete the |
@valentinpalkovic that's awesome! I also tested it in Typescript and it works great. There are two scenarios which will break with a similar error than before, but honestly I don't think people will do them, so I think it's fine to merge this! First scenario: an object of fonts:// fonts.ts
import { NextFontWithVariable } from '@next/font/dist/types';
import { Rubik_Puddles } from '@next/font/google';
import localFont from '@next/font/local';
export const fonts: Record<string, NextFontWithVariable> = {
rubik: Rubik_Puddles({
subsets: ['latin'],
variable: '--font-latin-rubik',
weight: '400',
}),
localRubikStorm: localFont({
src: '/fonts/RubikStorm-Regular.ttf',
variable: '--font-rubik-storm',
})
} // FontComponent.ts
import React from 'react';
import { fonts } from './fonts';
const { localRubikStorm, rubik } = fonts;
export default function Font({ variant }: {variant: string}) { ... } Second scenario: using the
|
@yannbf I am not sure whether the first scenario even works in Next.js. In their SWC code, they have a lot of if conditions where they throw errors, for example, if the import is aliased. Therefore I also think this is fine for now. Could you resolve the conflicts and merge the PR? (I think you can ignore the flaky windows unit tests.) I don't have unfortunately access to my machine right now. |
You're right! It does fail in Next.js: By switching it a little bit (moving to separate consts and exporting the fonts object, it works in Next.js and also in Storybook 🎉 |
🎉 |
🎉 INDEED! 👏 |
Thank so much for the update here @valentinpalkovic I'm using next This has now fixed the issue with Storybook not loading because of My font looks like this: src/styles/fonts.ts
And in my components i use
The file path actually looks correct but 404s.
Any ideas? Am i missing something? Thanks so much for any help. |
Hi @hobadams, Have you set up the |
@valentinpalkovic - you are a hero, thank you so much. That fixed it! 🎉 |
Can confirm @valentinpalkovic is a hero, for sure! |
hi @valentinpalkovic this change looks amazing, is there a way to use the fonts directly inside maybe preview.ts? I want to load all my fonts once and the use css to switch between them, without using them inside the components directly 😊 |
@patrick91 it shouldn’t be an issue. Just import the fonts, configure them and use for example a decorator, which sets the fonts onto a root element. Then use the classname or style property like described in the next font documentation. |
@valentinpalkovic thanks, I just got it working like this: // preview.ts
import "../src/styles/globals.css";
import localFont from "@next/font/local";
const ranade = localFont({
src: [
{
path: "../fonts/Ranade-Variable.ttf",
style: "normal",
},
{
path: "../fonts/Ranade-VariableItalic.ttf",
style: "italic",
},
],
variable: "--font-ranade",
// style: ["normal", "italic"],
});
document.body.classList.add(ranade.variable);
document.body.classList.add("font-sans");
export const parameters = {
backgrounds: {
default: "light",
},
actions: { argTypesRegex: "^on[A-Z].*" },
controls: {
matchers: {
color: /(background|color)$/i,
date: /Date$/,
},
},
}; will try the other options in future! <3 |
@patrick91 Great! If you need further assistance, you can reach out to the Storybook Discord Group at any time. We will try to give you the help and assistance you need. |
EDIT: I found the problem for my case, I'm using the latest import from NextJs Related: #21330 vercel/next.js#46159 I tried several options but I'm still stuck with the same error: and this is the error for local fonts: using Storybook 7.0.0-beta.61 and Next 13.2.3 |
@Cayan |
Issue: #19711
What I did
Implemented support of @next/font in Storybook.
1. Custom babel plugin to transform
@next/font
usages and prepare them for the webpack loaderI need to transform "@next/font" imports and usages to a webpack loader-friendly format with parameters.
The plugin turns the following code:
Into the following:
This Plugin tries to adopt the functionality provided by the nextjs swc plugin. Unfortunately, Next.js hasn't worked on a babel plugin. Indeed, if you opt out to use babel in Next.js, you are not allowed to use
@next/font
. Therefore this completely newly written plugin was necessary to adopt the same functionality in a babel environment.2. Custom
storybook-nextjs-font-loader
webpack loaderThe
storybook-nextjs-font-loader
does two things:@font-face
definitions into a style tag and places it into the head section of the document.className
,styles
andvariable
outputs to make them usable in React Components, like it is described in the docsThe transformed import
provides an object as a default export
How to test
@next/font/google
nextjs/default-js
sandbox@next/font/local
nextjs/default-js
sandbox.storybook/main.js
and add the following config:Supported packages
Not supported features in the first iteration:
How to test
If your answer is yes to any of these, please make sure to include it in your PR.