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

Broken when dev with type: module #22

Closed
yf-yang opened this issue Jun 18, 2024 · 7 comments · Fixed by #25
Closed

Broken when dev with type: module #22

yf-yang opened this issue Jun 18, 2024 · 7 comments · Fixed by #25
Labels
bug Something isn't working

Comments

@yf-yang
Copy link

yf-yang commented Jun 18, 2024

Reproduction

https://github.com/yf-yang/rsbuild-storybook-esm

Note: I added type: module in the package.json, others are remaining the same with the example

Error

Execute pnpm storybook
The server starts successfully, then each component throws "ReferenceError: require is not defined"

Additional resouces

Seems there is a similar old bug storybookjs/storybook#14877

@fi3ework fi3ework added the bug Something isn't working label Jun 19, 2024
@nate-summercook
Copy link

I can confirm the same issue on my side. Trying to migrate from webpack to rsbuild and using type: module. I also use module federation and the motivation to move to rsbuild is of course speed :)
Please fix this. From my investigations it seems that storybook imports the dist/index.js file instead of the dist/index.mjs which of course won't work. Why that is I don't know.

@yf-yang
Copy link
Author

yf-yang commented Jun 23, 2024

@nate-summercook @fi3ework

As a temporary workaround, I can use pnpm patch to solve the problem.
pnpm patch modifies a installed package's released code directly.

Add storybook-builder-rsbuild as dev dependency (That helps to find the package in node_modules)
Then,

diff --git a/templates/virtualModuleModernEntry.js.handlebars b/templates/virtualModuleModernEntry.js.handlebars
index 1224d3d015df76df07e8074d9328ae19174959ee..e5014d3741dfe116ccf0fae9273457966bf3dd71 100644
--- a/templates/virtualModuleModernEntry.js.handlebars
+++ b/templates/virtualModuleModernEntry.js.handlebars
@@ -5,8 +5,8 @@ import { createBrowserChannel } from '@storybook/channels';
 
 import { importFn } from './{{storiesFilename}}';
 
-const getProjectAnnotations = () =>
-  composeConfigs([{{#each previewAnnotations}}require('{{this}}'),{{/each}}]);
+const getProjectAnnotations = async () =>
+  composeConfigs([{{#each previewAnnotations}}await import('{{this}}'),{{/each}}]);
 
 const channel = createBrowserChannel({ page: 'preview' });
 addons.setChannel(channel);

I think the diff clearly illustrates the cause of the bug, but I don't know how the module type could be correctly pass to the template. If there are any suggestions I can offer a fix.

@yf-yang
Copy link
Author

yf-yang commented Jun 23, 2024

All right, I don't know how to load the package.json then pass to getVirtualModules. Waiting for the maintainer to fix/give instructions.

@fi3ework
Copy link
Member

I took a look into the code and took a test locally.

Using async writing work fine for module and commonjs type. I haven't test will webpack-builder will encounter the same issue which has the same require code now. Also wonder why storybookjs/storybook#18620 is reverted in the latest code.

const getProjectAnnotations = async () => {
  const configs = await Promise.all([{{#each previewAnnotations}}import('{{this}}'),{{/each}}])
  return composeConfigs(configs);
}

@fi3ework
Copy link
Member

  • If webpack-builder encounter the same issue, this should be also sync up to storybookjs/storybook.
  • If webpack-builder not encounter the same issue, we should figure out why it works in webpack, it might be a difference between Rspack and webpack.

@yf-yang
Copy link
Author

yf-yang commented Jun 24, 2024

Webpack + babel example (no issue):
https://github.com/yf-yang/rsbuild-storybook-esm/tree/webpack-test

Hope it helps.

@nate-summercook
Copy link

I can confirm that that error is gone, now I have different issues (#29 & #30)
help me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants