-
-
Notifications
You must be signed in to change notification settings - Fork 9.2k
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
v4.1.0 stop allowing webpack.config.js to influence manager #4995
Comments
@nowells I didn't expect people would need to modify the webpack config for manager. I was wrong. There's a solution for you, which you can do right now:
This should modify the webpack config of the manager to your desire. You can use this file to modify both configs at once: export async function managerWebpack(baseConfig, options) {
return baseConfig;
}
export async function managerBabel(baseConfig, options) {
return baseConfig;
}
export async function webpack(baseConfig, options) {
return baseConfig;
}
export async function babel(baseConfig, options) {
return baseConfig;
} We haven't really introduced this api to the world yet, but this should allow you to do what you want. |
Thank @ndelangen thanks for getting back to me. I looked through the code and found the Also, this appears to be a breaking change from the published docs for It seems to me that restoring the Thanks for your time and consideration of this issue. |
@ndelangen I tried your solution and got:
|
@ndelangen any guidance on my question above (sorry, not trying to push), we have pinned to |
@nowells Sorry for the inconvenience here. We'll try to get it sorted ASAP. First, a question: do you need a custom webpack config or if we added back in a bunch of standard loaders would that solve your problem in the short term? (The current manager config is pretty stripped down, but doesn't need to be.) @igor-dv At what point can we call the presets API stable? I think this is a pretty good use case for it--I don't see us going back to a unified config, and it looks like people are relying on being able to configure the manager too. See also #5004 |
@lflpowell: I was able to get it working with this:
|
@shilman we need a custom webpack config in our workflow, we need to run our own plugins to make storybooks work in our ecosystem. At the very least adding back in the standard loaders might help (such as JSX) however, I think having the ability to customize just like the preview is pretty powerful and useful for more advanced cases. |
@shilman since this also broke our storybook usage that is based on CRA (so not having any additional webpack config) we would be very happy if standard loaders would work out of the box when compiling the manager. We have e.g. an addon that now breaks the compile process since it is loading png files (or rather failing to import them now). |
@shilman, it's internally stable, but we didn't want to introduce a new |
+1 for this issue. Seems like it's a root cause of the one that I reported: #5122 When I install storybook 4.0.9 seems to be working fine. |
Here's what @igor-dv and I discussed as a proposed solution. Feedback appreciated @ndelangen @tmeasday @nowells @travikk @Laskoran @lflpowell @DBosley. If this sounds reasonable, PRs are welcome. If nobody takes it in the next few days, I'll take a swing at it myself. Long term We're planning a unified preview/manager config file, in which case you would simply configure your manager as you see fit. Unfortunately, we don't have a timeline on this yet. Short term This is an unintentional breaking change in 4.1, and to address the issue we propose a new package, titled something like Please let me know what you think! |
@shilman what does the package do exactly? My thinking (pardon me for the drive-by) was that if the only reason we didn't want to support It doesn't seem like such a back-compat burden to support it as a stand in for the |
@tmeasday When you install the package, it sets up the presets to make sure that your manager config matches your preview config. |
Sounds good to me. |
So I propose we fix this meantime like so: // .storybook/webpack.config.js
module.exports = {
preview: async ({ config }) => ({
...config,
}),
manager: async ({ config }) => ({
...config,
}),
}; Look like the cleanest solution, to me. I can work on this today and have a fix out by a few days. What do you think @shilman |
@ndelangen let's just use presets rather than complicating the API. I think this is a corner case. I'm working on the documentation now in #5301 |
[fix] Re-enable EX_CROSS_PLATFORM in load-examples.js [tiny ux] Use example index as a prefix in story name. [fix api] Implement preset-based solution based on storybookjs/storybook#4995 [api] Debug using `diagnostics` [refactor] Move reusable functionality from package/react/webpack.config.js to packages/react/{constants,resolve}.js
[fix] Re-enable EX_CROSS_PLATFORM in load-examples.js [tiny ux] Use example index as a prefix in story name. [fix api] Implement preset-based solution based on storybookjs/storybook#4995 [api] Debug using `diagnostics` [refactor] Move reusable functionality from package/react/webpack.config.js to packages/react/{constants,resolve}.js
[fix] Re-enable EX_CROSS_PLATFORM in load-examples.js [tiny ux] Use example index as a prefix in story name. [fix api] Implement preset-based solution based on storybookjs/storybook#4995 [api] Debug using `diagnostics` [refactor] Move reusable functionality from package/react/webpack.config.js to packages/react/{constants,resolve}.js
@mohsinulhaq Thanks, I'll remove that warning! |
Son of a gun!! I just released https://github.com/storybooks/storybook/releases/tag/v5.0.1 containing PR #5911 that references this issue. Upgrade today to try it out! Closing this issue. Please re-open if you think there's still more to do. |
thanks @shilman 🤩 |
Ooh-la-la!! I just released https://github.com/storybooks/storybook/releases/tag/v5.1.0-alpha.3 containing PR #5911 that references this issue. Upgrade today to try it out! Because it's a pre-release you can find it on the |
wow, is this an automated script? |
May I know why all these functions are marked async? I remove async and it still works. |
I just wanted to say thank you for your work in making the |
@mohsinulhaq There's a practice of annotating promise-returning functions as async even if they don't use |
wouldn't that result in wrapping promise inside another promise? |
You can't wrap JS promises, as they automatically flatten. |
Wow, I didn't know that. Thanks a lot @slikts ! |
It would be great to note in the docs that the customization of webpack does not include the manager, and other core webpack bundles. Total PITA to figure that out. |
We also need to override the manager webpack config so we can enforce a maximum chunk size. Seems like there has been some changes with preset config since this thread started. The solution that worked for SB 5.x.x. was to use a Background: since upgrading to SB 5.3.1 we have had issues with JS bundles greater than 1MB (constraint due to hosting on AWS S3 via lambda for auth). |
@johnhunter you should be able to do this by filling in |
All good thanks! main.js
|
@shilman I have an issue after migrating from 5.2.* to 5.3.*. I've tried to move to new configs, so my
There is an
Could you please advise me what could be the possible fix for that? Any help would be appreciated. |
@andrei-ilyukovich I believe this is fixed in #9648 |
how to make this work in storybook v6? |
Describe the bug
@storybook/core@4.1.0
broke (<= 4.0.12
worked) the ability for.storybook/webpack.config.js
to contribute to themanager
webpack bundle. This is important to us, as we have addons that rely on webpack features that we need to configure, this worked in versions before4.1.0
, however, it appears that this behavior was removed at that time.To Reproduce
Attempt to use a feature in your
.storybook/addon.js
that yourwebpack.config.js
enables.Expected behavior
I should be able to use features and settings in the
manager
bundle from the customwebpack.config.js
The text was updated successfully, but these errors were encountered: