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

@next/plugin-storybook issues #19345

Closed
7 tasks
eric-burel opened this issue Nov 20, 2020 · 10 comments
Closed
7 tasks

@next/plugin-storybook issues #19345

eric-burel opened this issue Nov 20, 2020 · 10 comments
Milestone

Comments

@eric-burel
Copy link
Contributor

eric-burel commented Nov 20, 2020

Bug report

To Reproduce

I've done a separate basic app, in which I try to make Storybook work with ONLY @next/plugin-storybook, to check its limitations/bugs step by step:

https://github.com/lbke/next-plugin-storybook-demo

All instructions to debug locally are listed there

I am also testing in in Vulcan Next, which is a realistic install.

yarn
yarn add @next/plugin-storybook
# Add @next/plugin-storybook in `.storybook/main.js`
yarn run storybook
# You'll get various error messages

Also PR #18367 demos a minimal install + a TypeScript version (but the TypeScript build never worked).

Describe the bug

I track here issues encountered with @next/plugin-storybook, in order to provide 1st class Storybook support.

TypeScript

Various

  • @next/plugin-storybook seems to have a hidden dependency to error-loader. What is the use of this loader? Edit: this issue has been mentioned here already: [@next/storybook] - incorrectly imported css throws an inaccurate error #15195. To sum it up this generic message is simply the sign that the build is not working as expected, and then the error can't be displayed correctly due to the lack of error-loader.
  • It doesn't like whenReact is imported again in a component (Next has some magic so import React from "react" is optional, but Storybook doesn't).
Syntax error: Identifier 'React' has already been declared

   9 | var _jsxFileName = "/code/vulcan-next-starter/README.md";
  10 | var __jsx = React.createElement;
> 11 | import React from 'react';
     |        ^
  12 | import { mdx } from '@mdx-js/react';
  13 | /* @jsx mdx */
  14 | export const frontMatter = {

ERROR in ./src/stories/Header.tsx:4:0
Module not found: Can't resolve 'error-loader'
  2 | 
  3 | import { Button } from './Button';
> 4 | import './header.css';
  5 | 
  6 | export interface HeaderProps {
  7 |   user?: {};

Module not found: Can't resolve '~/types/mdx.d.ts'
> 1 | import "~/types/mdx.d.ts"; // TODO: load this automatically
  2 | import Readme from "../../../README.md";
  3 | import { Typography } from "@material-ui/core";
  • See Cannot read property 'prefetch' and 'push' of null #16864 : @next/plugin-storybook should also include a decorator that loads the router in order to avoid any potential issues with components such as Link. The decorator proposed in this issue could also be exposed on its own, until we figure out all the plugin issue, so user can use just the decorator already.

Issues with Webpack 5 update and conflicts

WARN   Failed to load preset: {"name":"/code/next-plugin-storybook-demo/.yalc/@next/plugin-storybook/preset.js","type":"presets"} on level 1
ERR! TypeError: _chalk.default.constructor is not a constructor
ERR!     at Object.<anonymous> (/code/next-plugin-storybook-demo/.yalc/next/dist/build/webpack/plugins/wellknown-errors-plugin/parseBabel.js:1:277)
``
=> seems to be a version clash, forcing v2.4.2 in resolutions palliates the bug

- [ ] Similar with `jest-worker` Next expects v26 but Storybook v24, which have different APIs (no fixes here...) . This seems to happen when there is still a  Webpack 4 version.

- [ ] webpack < 5 used to include polyfills for node.js core modules by default.

This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:
- add a fallback 'resolve.fallback: { "crypto": require.resolve("crypto-browserify") }'
- install 'crypto-browserify'
If you don't want to include a polyfill, you can use an empty module like this:
resolve.fallback: { "crypto": false }


### Solved 
- [X] New error after updating to Webpack 5 (but seems to be on Storybook side https://github.com/storybookjs/storybook/issues/14257)

```TypeError: The 'compilation' argument must be an instance of Compilation
    at Function.getCompilationHooks (/home/eric-burel/code-hdd/next.js/packages/next/dist/compiled/webpack/bundle5.js:97586:10)
    at /home/eric-burel/code-hdd/next.js/packages/next/dist/build/webpack/plugins/mini-css-extract-plugin/src/index.js:137:84```

As far as I can tell the solution will be to yarn link ./node_modules/webpack in @next/plugin-storybook

=> Solved by using yalc for local development and linking packages/next-plugin-storybook and packages/next from Next canary branch

Expected behavior

The plugin works and handle paths aliases out of the box + some Next specific feature, like optional import React.

Expected minimum features:

  • dotenv support
  • CSS/styling solutions
  • Module aliases and path related features
  • Others?
@eric-burel eric-burel added the bug Issue was opened via the bug report template. label Nov 20, 2020
@eric-burel eric-burel changed the title @next/plugin-storybook fixes @next/plugin-storybook issues Nov 20, 2020
@stefanprobst
Copy link
Contributor

another issue is importing global css, which Next.js only allows in pages/_app, but which would need to also be allowed in .storybook/preview.

@eric-burel
Copy link
Contributor Author

Note that there is a sister issue in Storybook, to track more broadly the potential inconsistencies: storybookjs/storybook#9610

@weyert
Copy link

weyert commented Nov 22, 2020

I have the same issue that it can't resolve error-loader

@stefanprobst
Copy link
Contributor

for the error-loader issue see also #15195

@timneutkens timneutkens added type: experimental and removed bug Issue was opened via the bug report template. labels Nov 23, 2020
@timneutkens timneutkens added this to the backlog milestone Nov 23, 2020
@Timer Timer removed this from the backlog milestone Nov 23, 2020
@Timer Timer added this to the backlog milestone Jan 6, 2021
@Vadorequest
Copy link
Contributor

Vadorequest commented Jan 12, 2021

For follow-up, I successfully added support for CSS modules. (see storybookjs/storybook#9610 (comment))
Using https://www.npmjs.com/package/storybook-css-modules-preset basically fixed it for me.

See commit UnlyEd/next-right-now@777c3ce
See PR: UnlyEd/next-right-now#251

Storybook demo at https://nrn-v2-mst-aptd-at-lcz-sty-storybook.vercel.app/?path=/story/utilities-i18nlink--link-with-css-module

If you want to experiment with the implementation, you can clone the branch:

  1. git clone https://github.com/UnlyEd/next-right-now.git nrn-quick-start
  2. cd nrn-quick-start && git checkout storybook
  3. cp .env.local.example .env.local - Uses the default ENV variables when running locally
  4. yarn
  5. yarn sb

@eric-burel
Copy link
Contributor Author

I've finally cleaned up my open PR in favour of this one #23020

If merged, this will bring an official Storybook example that would include @next/plugin-storybook. From there, we can improve the configuration + also improve the plugin itself to fix remaining issues.

This work would also benefit any tool that want to make profit of the "Next.js magic" during build time (Cypress, Jest...).

@stefanprobst
Copy link
Contributor

since recently, it seems this is also required in

- rewrites: [],
+ rewrites: { beforeFiles: [], afterFiles: [], fallback: [] },

@eric-burel
Copy link
Contributor Author

since recently, it seems this is also required in

- rewrites: [],
+ rewrites: { beforeFiles: [], afterFiles: [], fallback: [] },

Indeed, PR sent: #24827

kodiakhq bot pushed a commit that referenced this issue May 6, 2021
This fixes a bug in `@next/plugin-storybook` caused by an update in the rewrite structure, solution was provided by @stefanprobst #19345 (comment)

Secondary question: it would be awesome if someone at Next could help translating `preset.js` into TypeScript, this would catch those bugs immediately. I've tried in #18367 but failed to build correctly.

## Bug

- [X] Related issues linked using `fixes #number` => parent issue is an umbrella of various small issues: #19345
- [X] Integration tests added => reproduction is implemented in https://github.com/lbke/next-plugin-storybook-demo until the package is more stable
flybayer pushed a commit to blitz-js/next.js that referenced this issue Jun 1, 2021
This fixes a bug in `@next/plugin-storybook` caused by an update in the rewrite structure, solution was provided by @stefanprobst vercel#19345 (comment)

Secondary question: it would be awesome if someone at Next could help translating `preset.js` into TypeScript, this would catch those bugs immediately. I've tried in vercel#18367 but failed to build correctly.

## Bug

- [X] Related issues linked using `fixes #number` => parent issue is an umbrella of various small issues: vercel#19345
- [X] Integration tests added => reproduction is implemented in https://github.com/lbke/next-plugin-storybook-demo until the package is more stable
@eric-burel
Copy link
Contributor Author

Closing as this experimental package is doom to fail in the way it is architectured (at the time of writing) because it uses the full Next.js webpack magic, instead we need to setup Storybook step by step.

@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants