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

Add an example using @next/plugin-storybook + fix webpack rules #23020

Closed
wants to merge 1 commit into from

Conversation

eric-burel
Copy link
Contributor

@eric-burel eric-burel commented Mar 12, 2021

This PR replaces #18367 , where I tried (but failed) to make a TS version of @next/plugin-storybook.

This PR:

Closes storybookjs/storybook#11639

@ijjk ijjk added the examples Issue/PR related to examples label Mar 12, 2021
@ijjk
Copy link
Member

ijjk commented Mar 12, 2021

Stats from current PR

Default Server Mode (Increase detected ⚠️)
General
vercel/next.js canary lbke/next.js storybook-alias-example Change
buildDuration 12.5s 12.3s -177ms
nodeModulesSize 42.8 MB 42.8 MB
Page Load Tests Overall increase ✓
vercel/next.js canary lbke/next.js storybook-alias-example Change
/ failed reqs 0 0
/ total time (seconds) 2.308 2.258 -0.05
/ avg req/sec 1083.12 1107.22 +24.1
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 1.548 1.563 ⚠️ +0.01
/error-in-render avg req/sec 1615.46 1599.21 ⚠️ -16.25
Client Bundles (main, webpack, commons)
vercel/next.js canary lbke/next.js storybook-alias-example Change
677f882d2ed8..a2e7.js gzip 13.4 kB 13.4 kB
framework.HASH.js gzip 39 kB 39 kB
main-HASH.js gzip 6.65 kB 6.65 kB
webpack-HASH.js gzip 751 B 751 B
Overall change 59.8 kB 59.8 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary lbke/next.js storybook-alias-example Change
polyfills-HASH.js gzip 31.3 kB 31.3 kB
Overall change 31.3 kB 31.3 kB
Client Pages
vercel/next.js canary lbke/next.js storybook-alias-example Change
_app-fde3324..9dd1.js gzip 1.28 kB 1.28 kB
_error-af59f..582f.js gzip 3.46 kB 3.46 kB
amp-9716187d..0aa8.js gzip 536 B 536 B
hooks-107e90..74c7.js gzip 888 B 888 B
index-ac435c..ecf2.js gzip 227 B 227 B
link-c0d2c96..de48.js gzip 1.67 kB 1.67 kB
routerDirect..dc9d.js gzip 303 B 303 B
withRouter-6..0e02.js gzip 302 B 302 B
Overall change 8.66 kB 8.66 kB
Client Build Manifests
vercel/next.js canary lbke/next.js storybook-alias-example Change
_buildManifest.js gzip 347 B 347 B
Overall change 347 B 347 B
Rendered Page Sizes
vercel/next.js canary lbke/next.js storybook-alias-example Change
index.html gzip 612 B 612 B
link.html gzip 620 B 620 B
withRouter.html gzip 608 B 608 B
Overall change 1.84 kB 1.84 kB

Serverless Mode
General
vercel/next.js canary lbke/next.js storybook-alias-example Change
buildDuration 14.2s 13.8s -334ms
nodeModulesSize 42.8 MB 42.8 MB
Client Bundles (main, webpack, commons)
vercel/next.js canary lbke/next.js storybook-alias-example Change
677f882d2ed8..a2e7.js gzip 13.4 kB 13.4 kB
framework.HASH.js gzip 39 kB 39 kB
main-HASH.js gzip 6.65 kB 6.65 kB
webpack-HASH.js gzip 751 B 751 B
Overall change 59.8 kB 59.8 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary lbke/next.js storybook-alias-example Change
polyfills-HASH.js gzip 31.3 kB 31.3 kB
Overall change 31.3 kB 31.3 kB
Client Pages
vercel/next.js canary lbke/next.js storybook-alias-example Change
_app-fde3324..9dd1.js gzip 1.28 kB 1.28 kB
_error-af59f..582f.js gzip 3.46 kB 3.46 kB
amp-9716187d..0aa8.js gzip 536 B 536 B
hooks-107e90..74c7.js gzip 888 B 888 B
index-ac435c..ecf2.js gzip 227 B 227 B
link-c0d2c96..de48.js gzip 1.67 kB 1.67 kB
routerDirect..dc9d.js gzip 303 B 303 B
withRouter-6..0e02.js gzip 302 B 302 B
Overall change 8.66 kB 8.66 kB
Client Build Manifests
vercel/next.js canary lbke/next.js storybook-alias-example Change
_buildManifest.js gzip 347 B 347 B
Overall change 347 B 347 B
Serverless bundles
vercel/next.js canary lbke/next.js storybook-alias-example Change
_error.js 1.02 MB 1.02 MB
404.html 2.67 kB 2.67 kB
500.html 2.65 kB 2.65 kB
amp.amp.html 10.6 kB 10.6 kB
amp.html 1.86 kB 1.86 kB
hooks.html 1.92 kB 1.92 kB
index.js 1.02 MB 1.02 MB
link.js 1.08 MB 1.08 MB
routerDirect.js 1.07 MB 1.07 MB
withRouter.js 1.07 MB 1.07 MB
Overall change 5.27 MB 5.27 MB

Webpack 5 Mode (Decrease detected ✓)
General
vercel/next.js canary lbke/next.js storybook-alias-example Change
buildDuration 14.6s 14.9s ⚠️ +312ms
nodeModulesSize 42.8 MB 42.8 MB
Page Load Tests Overall decrease ⚠️
vercel/next.js canary lbke/next.js storybook-alias-example Change
/ failed reqs 0 0
/ total time (seconds) 2.215 2.316 ⚠️ +0.1
/ avg req/sec 1128.62 1079.36 ⚠️ -49.26
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 1.451 1.548 ⚠️ +0.1
/error-in-render avg req/sec 1722.98 1615.17 ⚠️ -107.81
Client Bundles (main, webpack, commons)
vercel/next.js canary lbke/next.js storybook-alias-example Change
597-2bc2376a..203d.js gzip 13.3 kB 13.3 kB
framework.HASH.js gzip 39.3 kB 39.3 kB
main-HASH.js gzip 6.59 kB 6.59 kB
webpack-HASH.js gzip 954 B 954 B
Overall change 60.2 kB 60.2 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary lbke/next.js storybook-alias-example Change
polyfills-HASH.js gzip 31.1 kB 31.1 kB
Overall change 31.1 kB 31.1 kB
Client Pages
vercel/next.js canary lbke/next.js storybook-alias-example Change
_app-0c62a59..94b7.js gzip 1.26 kB 1.26 kB
_error-97d24..ed28.js gzip 3.38 kB 3.38 kB
amp-2926e4c2..9ccc.js gzip 536 B 536 B
hooks-1ed65b..8908.js gzip 902 B 902 B
index-6259b6..77d8.js gzip 230 B 230 B
link-b722682..14a4.js gzip 1.65 kB 1.65 kB
routerDirect..862a.js gzip 306 B 306 B
withRouter-4..76fd.js gzip 302 B 302 B
Overall change 8.57 kB 8.57 kB
Client Build Manifests
vercel/next.js canary lbke/next.js storybook-alias-example Change
_buildManifest.js gzip 322 B 322 B
Overall change 322 B 322 B
Rendered Page Sizes
vercel/next.js canary lbke/next.js storybook-alias-example Change
index.html gzip 586 B 586 B
link.html gzip 593 B 593 B
withRouter.html gzip 579 B 579 B
Overall change 1.76 kB 1.76 kB

Diffs

Diff for index.html
@@ -43,7 +43,7 @@
         "props": { "pageProps": {} },
         "page": "/",
         "query": {},
-        "buildId": "L346usyHMHJ0eGPxv8Fev",
+        "buildId": "fbhb47do3jycyktUWYpog",
         "isFallback": false,
         "gip": true
       }
@@ -77,11 +77,11 @@
       async=""
     ></script>
     <script
-      src="/_next/static/L346usyHMHJ0eGPxv8Fev/_buildManifest.js"
+      src="/_next/static/fbhb47do3jycyktUWYpog/_buildManifest.js"
       async=""
     ></script>
     <script
-      src="/_next/static/L346usyHMHJ0eGPxv8Fev/_ssgManifest.js"
+      src="/_next/static/fbhb47do3jycyktUWYpog/_ssgManifest.js"
       async=""
     ></script>
   </body>
Diff for link.html
@@ -48,7 +48,7 @@
         "props": { "pageProps": {} },
         "page": "/link",
         "query": {},
-        "buildId": "L346usyHMHJ0eGPxv8Fev",
+        "buildId": "fbhb47do3jycyktUWYpog",
         "isFallback": false,
         "gip": true
       }
@@ -82,11 +82,11 @@
       async=""
     ></script>
     <script
-      src="/_next/static/L346usyHMHJ0eGPxv8Fev/_buildManifest.js"
+      src="/_next/static/fbhb47do3jycyktUWYpog/_buildManifest.js"
       async=""
     ></script>
     <script
-      src="/_next/static/L346usyHMHJ0eGPxv8Fev/_ssgManifest.js"
+      src="/_next/static/fbhb47do3jycyktUWYpog/_ssgManifest.js"
       async=""
     ></script>
   </body>
Diff for withRouter.html
@@ -43,7 +43,7 @@
         "props": { "pageProps": {} },
         "page": "/withRouter",
         "query": {},
-        "buildId": "L346usyHMHJ0eGPxv8Fev",
+        "buildId": "fbhb47do3jycyktUWYpog",
         "isFallback": false,
         "gip": true
       }
@@ -77,11 +77,11 @@
       async=""
     ></script>
     <script
-      src="/_next/static/L346usyHMHJ0eGPxv8Fev/_buildManifest.js"
+      src="/_next/static/fbhb47do3jycyktUWYpog/_buildManifest.js"
       async=""
     ></script>
     <script
-      src="/_next/static/L346usyHMHJ0eGPxv8Fev/_ssgManifest.js"
+      src="/_next/static/fbhb47do3jycyktUWYpog/_ssgManifest.js"
       async=""
     ></script>
   </body>
Commit: 2c8db40

@stefanprobst
Copy link
Contributor

for the first point also see #22125

@matamatanot
Copy link
Contributor

matamatanot commented Apr 23, 2021

@Timer
#20759
It looks like most of the plugins have been removed, but does this affect @next/plugin-storybook too?

@eric-burel
Copy link
Contributor Author

Btw that's something we could take care of in Storybook instead or as a community package. The interesting part of having it in Next is that we can access webpack code directly, because it could benefit for slight refactor to make it easier to reuse the "build magic" of Next in 3rd party tools (Jest, Storybook...). Storybook is a good use case for this.

@Sharcoux
Copy link
Contributor

@divmain @ijjk @lfades @shuding @timneutkens There is this open PR and that one fixing an implementation that everyone can see was wrong, and without any impact I can think of except than making a broken implementation work again. Is there an extra concern I'm missing here? Otherwise this should probably be merged without delay.

@leerob
Copy link
Member

leerob commented Apr 23, 2021

I was wrong - my bad 😄

@leerob leerob reopened this Apr 23, 2021
@leerob
Copy link
Member

leerob commented Apr 23, 2021

Wouldn't it make more sense to just fix the current implementation for now with the current fix proposed here, and, when the new plugin system is ready, remove the legacy one? Or did I misunderstand?

This doesn't use that plugin system 😄 I misread this PR and thought it did, but it does not.

The answer to #23020 (comment) is no, it does not affect this 😄

@Sharcoux
Copy link
Contributor

@leerob now that you're here, can't this be merged?

@Sharcoux
Copy link
Contributor

@leerob please ?

@imns1ght
Copy link

I would be very grateful if someone merges this, I need these corrections too.

@Sharcoux

This comment has been minimized.

@eric-burel
Copy link
Contributor Author

eric-burel commented May 4, 2021

For those "in need" for this PR, though I also would like to see it merged as soon as possible, being the author of said PR, please keep in mind that this package is completely experimental anyway. And like really experimental, there are multiple known limitations to it (#19345). It may need multiple additional PR on Next.js webpack config to be fully working as expected.
The point of @next/plugin-storybook is not to make Storybook install possible (it already is), it is to make it a one-liner, which is a very different goal. It's ok if it takes time.
If you need Storybook in Next, there is a basic example in the official example folder, an example setup in Next Right Now (fairly complete with CSS modules and all), and an example setup in my own boilerplate Vulcan Next.
(I also probably added to the confusion by creating a first dirtier version of this PR that was using TypeScript but didn't work as expected)

@Sharcoux

This comment has been minimized.

@leerob
Copy link
Member

leerob commented May 4, 2021

@Sharcoux I'm marking your component as disruptive again, please consider using friendlier language.


I'm happy to merge #22125 which solves the same issue (just approved it!) – but I'm going to close this as I don't think we should add an example showing how to use an experimental plugin. This will lead to users trying to use this plugin, thinking it's ready for production, and being dissatisfied.

Once the plugin is stable, I would love to see the other Storybook example updated to use it 😄 🚀

@leerob leerob closed this May 4, 2021
@eric-burel
Copy link
Contributor Author

Makes sense, thanks for merging the fix PR from #22125. I am going to maintain the example there until @next/plugin-storybook is more stable: https://github.com/lbke/next-plugin-storybook-demo

@eric-burel
Copy link
Contributor Author

eric-burel commented May 5, 2021

@leerob just a question, I've never managed to turn the @next/plugin-storybook into TypeScript code. This would have caught the bug very early on, but somehow the imports as they are written now do not work when I use TypeScript (see my experiment in #18367, file packages/next-plugin-storybook/preset.ts). If you have someone at Next that could give some hints that could help stabilizing this plugin a bit.

@leerob
Copy link
Member

leerob commented May 5, 2021

@eric-burel I'm unfortunately a TypeScript novice so I can't be of much assistance. It's still magic to me 😄

@eric-burel
Copy link
Contributor Author

Hi, for those following this work, I'd need help on this issue: #25557
The plugin basically doesn't work anymore with webpack 5 (or I couldn't make it work at least), because Next.js is using it's own bundled version of Webpack, that creates conflict with Storybook own version.

Solution would lie into being able to reuse Next.js bundle, I am pretty sure it's technically possible but that's definitely too big for me alone, so any help is welcome.

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

Successfully merging this pull request may close these issues.

Storybook is not able to resolve path with Next js absolute import
7 participants