search

nuxt-modules / storybook

Storybook integration with Nuxt.
https://storybook.nuxtjs.org
411 stars 95 forks source link

fix: remove Nuxt context conflict #723

Closed obulat closed 3 months ago

obulat commented 4 months ago

🔗 Linked issue

Related #661

❓ Type of change

📚 Description

This PR updates the global nuxt context, setting its nuxt property to the story's nuxt when setting up the current story.

Previously, when navigating from one story to another story, Nuxt would get the context using the app's _name, which is the same for all stories, instead of using the globalName, which is different for each story.

netlify[bot] commented 4 months ago

Deploy request for nuxt-storybook pending review.

Visit the deploys page to approve it

Name Link
Latest commit 9c3f84baa7240c18ade7dfa39a0bde0aa3b5ac0a
CarlosZoft commented 4 months ago

@tobiasdiez

image

CarlosZoft commented 4 months ago

@obulat Do you have a temporary solution to this problem?

obulat commented 4 months ago

@obulat Do you have a temporary solution to this problem?

No, unfortunately I don't.

tobiasdiez commented 4 months ago

Awesome! On a first glance it looks good to me, will do a bit more testing in a few days and then merge.

guim4dev commented 4 months ago

kermit

tobiasdiez commented 4 months ago

Initially, each story in my code had its own Nuxt instance because each story functioned as a separate Vue app mounted in the Storybook canvas.

That's still the case. But nuxt has now initial support for multi-app projects, so the global context didn't seem to be necessary anymore.

@obulat what exactly is the need for the global context now? Do you know if

Nuxt would get the context using the app's _name, which is the same for all stories

is by design or may be a nuxt bug?

obulat commented 4 months ago

@obulat what exactly is the need for the global context now? Do you know if

Nuxt would get the context using the app's _name, which is the same for all stories

is by design or may be a nuxt bug?

From my research globalName was introduced in Nuxt 2 and the intention was to have it removed in Nuxt 3. globalName is still present in Nuxt3 code, but I think it's not used for anything.

._name was introduced in a recent PR that, as you say, @tobiasdiez, started enabling multi-app support in Nuxt: feat(nuxt): allows the support of multiple shared runtime context

By default, it is taken from '#build/nuxt.config.mjs':

https://github.com/nuxt/nuxt/blob/8abb1df01783876c3a39a41f24a4f86b12dd81ae/packages/nuxt/src/app/nuxt.ts#L24

Is it possible to set the Nuxt options' appId when setting up the Nuxt app in the preview.ts? Or would it be better to suggest to Nuxt to make the _name configurable in createNuxtApp call:

https://github.com/nuxt/nuxt/blob/8abb1df01783876c3a39a41f24a4f86b12dd81ae/packages/nuxt/src/app/nuxt.ts#L251

tobiasdiez commented 3 months ago

@obulat Thanks a lot for the investigation. Yesterday, I've created two PRs on the nuxt repo that should implement your findings: https://github.com/nuxt/nuxt/pull/28391 and https://github.com/nuxt/nuxt/pull/28392.

In the meantime we go with your solution, as it is clearly an improvement. I still get context issues in the "docs" section of the storybook, when multiple controls are displayed there. But single stories are working fine.

obulat commented 3 months ago

@obulat Thanks a lot for the investigation. Yesterday, I've created two PRs on the nuxt repo that should implement your findings: nuxt/nuxt#28391 and nuxt/nuxt#28392.

In the meantime we go with your solution, as it is clearly an improvement. I still get context issues in the "docs" section of the storybook, when multiple controls are displayed there. But single stories are working fine.

Thank you for all your work investigating and opening new upstream PRs, @tobiasdiez !