Skip to content
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

Use stable Next.js v13.2.0 next/font #21247

Merged
merged 13 commits into from Mar 2, 2023

Conversation

alex-ahumada
Copy link
Contributor

@alex-ahumada alex-ahumada commented Feb 24, 2023

Closes #21330

What I did

Migrated @next/font to stable next/font present in next.js v13.2.0.

How to test

  1. Add Storybook to a project using nextjs framework
  2. Add next/font to preview.js (example using css variables)
import { Roboto } from 'next/font/google'

...

const fontSans = Roboto({
  weight: ['400', '700'],
  style: ['normal', 'italic'],
  variable: '--font-roboto',
  subsets: ['latin'],
})

...

export const decorators = [
  (StoryFn) => (
      <div className={fontSans.variable}>
        <StoryFn />
      </div>
  ),
]

Checklist

  • Make sure your changes are tested (stories and/or unit, integration, or end-to-end tests)
  • Make sure to add/update documentation regarding your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Maintainers

  • If this PR should be tested against many or all sandboxes,
    make sure to add the ci:merged or ci:daily GH label to it.
  • Make sure this PR contains one of the labels below.

@valentinpalkovic
Copy link
Contributor

Hi @alex-ahumada

Thank you so much for your contribution!

I like what you have done! Though, we have to make the changes less disruptive. I am ok with changing docs and examples immediately from @next/font to next/font; but we still have to support both variants! (@next/font and next/font). Can you adjust your PR to not drop the support for @next/font?

@valentinpalkovic valentinpalkovic removed their assignment Feb 27, 2023
@alex-ahumada
Copy link
Contributor Author

Hi @valentinpalkovic .

No problem! Last commit should bring @next/font support back.

@valentinpalkovic
Copy link
Contributor

You forgot to push the yarn.lock file. :)

@shilman shilman added the nextjs label Feb 27, 2023
@alex-ahumada
Copy link
Contributor Author

ci/circleci: check fails, there are some problems with usePathName and useSearchParams from next/navigation after next.js v13.2.x update. Used in Navigation.stories.tsx

Related issue: vercel/next.js#46360

@valentinpalkovic
Copy link
Contributor

#21304 Got merged 3 hours ago. Please merge next into your branch one more time :)

@alex-ahumada
Copy link
Contributor Author

It seems we also have a missing headRenderedAboveThisLevel property in src/routing/app-router-provider.tsx(81,11)

@valentinpalkovic valentinpalkovic added the ci:daily Run the CI jobs that normally run in the daily job. label Mar 2, 2023
@valentinpalkovic valentinpalkovic merged commit 0f3e9e5 into storybookjs:next Mar 2, 2023
@valentinpalkovic
Copy link
Contributor

Well done @alex-ahumada! 🎉

@alex-ahumada
Copy link
Contributor Author

Thanks @valentinpalkovic ! Great teamwork! 😄

@Cayan
Copy link

Cayan commented Mar 6, 2023

I still see the import error when I import next/font instead of @next/font

Storybook version: 7.0.0-beta.61
Next.js version: 13.2.3

Error: (0 , next_font_google__WEBPACK_IMPORTED_MODULE_3__.Inter) is not a function
Similar errors for local fonts.

@valentinpalkovic
Copy link
Contributor

@Cayan can you provide a reproduction?

@Cayan
Copy link

Cayan commented Mar 6, 2023

I can create one later today @valentinpalkovic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug ci:daily Run the CI jobs that normally run in the daily job. nextjs
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Feature Request]: Support the package next/font (the replacement of @next/font)
4 participants