fix: use derived storybookConfig as the merge base #740
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
👷 Deploy request for nuxt-storybook pending review.Visit the deploys page to approve it
|
Signed-off-by: Ryan Leckey <[email protected]>
mehcode
force-pushed
the
rl-config-merge
branch
from
3428b6d to
f149e7a
Compare
|
Thanks! The changes look good to me, but the playground example ci is now failing. Could you please have a look? |
|
It's failing because now the typed documentation is being generated and that takes a serious amount of RAM. I'm away now but I'll try and circle back and add a NODE_OPTION to the command to up the space allowed for node. |
|
Even with increased heap size it fails (and increases the build time). Could you perhaps revert the type changes for now? |
tobiasdiez
added
the
status: needs info/work
|
I had a look at this and it turns out that it's the vue runtime compiler that is the problem. I "fixed" it by removing the vue alias, but now some of the showcase examples are empty. Do you have an idea what's the reason for the out of memory error? For me it always stops at (with the vue alias in place) but I'm not sure if that's indeed the true problematic file. Might be fixed with storybookjs/storybook#28589, although that seems to be a different problem. |
|
Any news about the status? |
|
I'm currently waiting for storybook 8.3.0 to see if storybookjs/storybook#28589 helped here. If you want to help: revert 25e2ac5 and investigate why we run out of memory during the build of the playground |
I will check it the next few days. Storybook 8.3.0 is released 🥳🥳 |
|
Sadly the update to Storybook 8.3 didn't rosolved the issue. I tried to investigate it further and it seems that (and the next time with one more appended) It's triggered by every import of vue, the first one being in |
|
@maximilian-schwarz did you already found the time to look into this a bit more? |
This comment was marked as off-topic.
This comment was marked as off-topic.
|
[
{
name: 'storybook:code-generator-plugin',
enforce: 'pre',
configureServer: [Function: configureServer],
config: [Function: config],
configResolved: [Function: configResolved],
resolveId: [Function: resolveId],
load: [AsyncFunction: load],
transformIndexHtml: [AsyncFunction: transformIndexHtml]
},
{
name: 'plugin-csf',
rollup: [Object],
vite: [Object],
webpack: [Function: webpack],
rspack: [Function: rspack],
enforce: 'pre',
transform: [AsyncFunction: transform]
},
{
name: 'storybook:inject-export-order-plugin',
enforce: 'post',
transform: [AsyncFunction: transform]
},
{
name: 'storybook:strip-hmr-boundary-plugin',
enforce: 'post',
transform: [AsyncFunction: transform]
},
{
name: 'storybook:allow-storybook-dir',
enforce: 'post',
config: [Function: config]
},
{
name: 'storybook:external-globals-plugin',
enforce: 'post',
config: [AsyncFunction: config],
transform: [AsyncFunction: transform]
},
{
name: 'storybook:rollup-plugin-webpack-stats',
enforce: 'post',
moduleParsed: [Function: moduleParsed],
storybookGetStats: [Function: storybookGetStats]
},
Promise { [Object] },
{
name: 'storybook:vue-docgen-plugin',
transform: [AsyncFunction: transform]
}
]Only the last 2 entries are different from import {
...
type Plugin as VitePlugin,
...
} from 'vite'
...
storybookViteConfig.plugins = storybookViteConfig.plugins.filter((plugin: VitePlugin) => !(plugin instanceof Promise)) |
aaron-adusa
reviewed
Oct 15, 2024
playground/.storybook/main.ts
Outdated
| @@ -15,9 +15,6 @@ const config: StorybookConfig = { | |||
| name: '@storybook-vue/nuxt', | |||
| options: {}, | |||
| }, | |||
| docs: { | |||
There was a problem hiding this comment.
Shouldn't this go in the playground/.storybook/main.ts now according to https://storybook.js.org/docs/writing-docs/autodocs#set-up-automated-documentation?
|
Out of curiosity, has this work been tested with the latest alpha release? I'm hoping that it'll be released in the upcoming weeks and it would be nice to have the related issues resolved. |
|
Any success with this ? |
8 tasks
8 tasks
|
Unfortunately, this does not solve #753. I tried to build your branch and patch |
|
@philip-hartmann Thanks for your investigation! It turns out that it was not actually the fact that the plugin was promised, but the plugin itself was the problem. I've removed it for now and everything is working well. @mehcode Thanks for your patience! |
tobiasdiez
added a commit
that referenced
this pull request
Dec 27, 2024
<!-- ☝️ PR title should follow conventional commits (https://conventionalcommits.org). In particular, the title should start with one of the following types: - docs: 📖 Documentation (updates to the documentation or readme) - fix: 🐞 Bug fix (a non-breaking change that fixes an issue) - feat: ✨ New feature/enhancement (a non-breaking change that adds functionality or improves existing one) - feat!/fix!:⚠️ Breaking change (fix or feature that would cause existing functionality to change) - chore: 🧹 Chore (updates to the build process or auxiliary tools and libraries) --> ### 🔗 Linked issue Fixes #808 and fixes #805. <!-- If it resolves an open issue, please link the issue here. For example "Resolves #123" --> ### 📚 Description <!-- Describe your changes in detail --> <!-- Why is this change required? What problem does it solve? --> This PR reverts #740 that removes of the vue-templation-template plugin, which was initially extended from the vue3-vite project vite config. However, removing it only masked the memory issue rather than addressing its root cause—the resolution of the vue ESM bundler alias. The vue-templation-template plugin is essential for rendering stories with custom string templates, so we need to retain it. To work around the memory issue and ensure template compilation works correctly, i have added vue as a dependency in the root project. ( playground ) The underlying problem appears to be related to how the vue alias is resolved. When it isn’t found in the root project, some unusual string replacement occurs, leading to recursive paths and the memory issue. Further investigation may be needed to fully understand why vue cannot be resolved from sub-packages. --------- Co-authored-by: Tobias Diez <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
anyfromtypes.d.tsstorybookConfigover the bareconfigwhen mergingShould fix #756 and fix #799