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

[Bug]: Next.js 13 - Broken next/link styles due to missing html attributes on <a> #19813

Closed
muhammad-saleh opened this issue Nov 11, 2022 · 11 comments

Comments

@muhammad-saleh
Copy link

Describe the bug

[Reporduction repo available]
https://github.com/muhammad-saleh/storybook-next13-link

We were migrating one of our projects from Next.js 12 to 13, one of the new changes is a new next/link component. We use Chromatic to detect visual regressions. Migration was smooth until we found regressions with links.
After investigation we found that Storybook doesn't emit props passed to the new next/link component, resulting in the anchors being rendered without styles.

Example of a button rendered by the app:
image

HTML:
image

Same component rendered in Storybook:
image

HTML:
image

To Reproduce

Repository:
https://github.com/muhammad-saleh/storybook-next13-link

Steps to reproduce:
1. Clone the repo
2. Run `yarn`
3. Run `yarn dev`, app should be on port 3000
4. Run separately `yarn storybook`
5. Compare the component on the homepage vs the one in Storybook

System

System:
    OS: Linux 6.0 Fedora Linux 36 (Workstation Edition)
    CPU: (12) x64 Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
  Binaries:
    Node: 16.13.1 - /usr/local/bin/node
    Yarn: 1.22.19 - ~/.yarn/bin/yarn
    npm: 8.17.0 - ~/.npm-global/bin/npm
  Browsers:
    Chrome: 107.0.5304.110
    Firefox: 106.0.4
  npmPackages:
    @storybook/addon-actions: ^6.5.13 => 6.5.13
    @storybook/addon-essentials: ^6.5.13 => 6.5.13
    @storybook/addon-interactions: ^6.5.13 => 6.5.13
    @storybook/addon-links: ^6.5.13 => 6.5.13
    @storybook/builder-webpack5: ^6.5.13 => 6.5.13
    @storybook/manager-webpack5: ^6.5.13 => 6.5.13
    @storybook/react: ^6.5.13 => 6.5.13
    @storybook/testing-library: ^0.0.13 => 0.0.13

Additional context

No response

@muhammad-saleh
Copy link
Author

As a workaround, I added this to preview.js to destructure the props to a child anchor. I'm still not sure if this will cause regressions or if this will cause Storybook/Chromatic to diverge from the main app.

Object.defineProperty(Link, 'default', {
  configurable: true,
  value: (props) => <OriginalLink {...props}><a {...props}>{props.children}</a></OriginalLink>,
})

@daviddelusenet
Copy link

daviddelusenet commented Nov 17, 2022

I can confirm that this is an issue, there are multiple issues with the next/link from Next 13. If you put anything else then a string inside of the Link component it won't render as a <a>.

@pm0u
Copy link

pm0u commented Nov 18, 2022

Also confirmed here, this was driving me crazy. Tested the "mappings" functionality to pass JSX as options as well that I saw referenced somewhere and the problem still persisted. If anything other than a string/text node is passed it fails to render the anchor. There doesn't seem to be any useful console output other than a warning about the react 17/18 root API from the storybook manager.

next 13.0.3
react 18.2.0
storybook/react 6.5.13

@pm0u
Copy link

pm0u commented Nov 18, 2022

@muhammad-saleh 's fix worked for me THANKS! - the full fix I needed:

// .storybook/preview.js
import * as NextLink from 'next/link';

const OriginalLink = NextLink.default;

Object.defineProperty(NextLink, 'default', {
  configurable: true,
  value: (props) => <OriginalLink {...props}><a {...props}>{props.children}</a></OriginalLink>,
})

@pauloamgomes
Copy link

I got Next 13 Link working on SB by setting in the .env file __NEXT_NEW_LINK_BEHAVIOR=true or enforcing the value as below (required for example in Netlify as the we can't set env vars prefixed with __):

// .storybook/main.js
webpackFinal: async (config) => {
    (...)
    config.plugins.push(
      new webpack.DefinePlugin({
        'process.env.__NEXT_NEW_LINK_BEHAVIOR': true,
      })
    )

    return config
  },

@pm0u
Copy link

pm0u commented Nov 18, 2022

@pauloamgomes do you have any more information on what exactly the flag does? i tried to track that down but it was pretty hard to follow. Are you setting that flag in the NextJS app as well?

@yannbf
Copy link
Member

yannbf commented Nov 18, 2022

@pm0u it's super tricky indeed to track it down. You can see explanation here: vercel/next.js#36436, and it's equivalent of next.config.js -> experimental.newNextLinkBehavior

There's an open PR at #19834 which will take care of this (for Next.js 12 projects that use the flag, or Next.js 13 projects by default). Will soon be available in the next versions of Storybook 7!

@yannbf yannbf mentioned this issue Nov 18, 2022
3 tasks
@pm0u
Copy link

pm0u commented Nov 18, 2022

@yannbf thanks. So, from what I gather this is not yet the default in Next13? That seems odd, since in use it does seem to be using the new link syntax in my NextJS app (13.0.3), which is the same version of Next used in my storybook (confirmed in the yarn.lock) without this ENV var set?

@kylegach
Copy link
Contributor

So, from what I gather this is not yet the default in Next13?

Yes, the new link behavior is the default in Next13. But they engineered it in a way such that the behavior is dependent on that env var, which they set automatically within a Next app and must be manually applied when used outside of Next (which our PR does).

@pm0u
Copy link

pm0u commented Nov 18, 2022

So, from what I gather this is not yet the default in Next13?

Yes, the new link behavior is the default in Next13. But they engineered it in a way such that the behavior is dependent on that env var, which they set automatically within a Next app and must be manually applied when used outside of Next (which our PR does).

Thank you for the explanation & fix! 🥇 that helps me a lot to increase confidence in working in storybook. cheers!

@shilman
Copy link
Member

shilman commented Nov 21, 2022

Hurrah!! I just released https://github.com/storybookjs/storybook/releases/tag/v7.0.0-alpha.52 containing PR #19834 that references this issue. Upgrade today to the @next NPM tag to try it out!

npx sb upgrade --prerelease

Closing this issue. Please re-open if you think there's still more to do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants