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

fix(plugin-nuxt): remove useNuxtApp mock in nuxt plugin preventing some behaviors (fix #703) #710

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

juleshry
Copy link

Description

This PR fixes the issue #703. This bug comes from the useNuxtApp that is mocked in /packages/histoire-plugin-nuxt/runtime/composables.mjs
It just removes this mock to use the useNuxtApp version from Nuxt. It allows some plugins that requires data from useNuxtApp to run correctly (e.g. @nuxtjs/i18n).


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the Commit Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

…viors

remove useNuxtApp mock since it prevents some plugins to run (e.g. @nuxtjs/i18n). It fixes the issue histoire-dev#703 .
Copy link

codesandbox bot commented Apr 12, 2024

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

Copy link

stackblitz bot commented Apr 12, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

Copy link

netlify bot commented Apr 12, 2024

Deploy Preview for histoire-examples-svelte3 ready!

Name Link
🔨 Latest commit 5cd8fd6
🔍 Latest deploy log https://app.netlify.com/sites/histoire-examples-svelte3/deploys/66a0b91812e8fc0008ffc657
😎 Deploy Preview https://deploy-preview-710--histoire-examples-svelte3.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Apr 12, 2024

Deploy Preview for histoire-controls ready!

Name Link
🔨 Latest commit 5cd8fd6
🔍 Latest deploy log https://app.netlify.com/sites/histoire-controls/deploys/66a0b918085236000875337a
😎 Deploy Preview https://deploy-preview-710--histoire-controls.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Apr 12, 2024

Deploy Preview for histoire-examples-vue3 ready!

Name Link
🔨 Latest commit 5cd8fd6
🔍 Latest deploy log https://app.netlify.com/sites/histoire-examples-vue3/deploys/66a0b91820e5c2000909baed
😎 Deploy Preview https://deploy-preview-710--histoire-examples-vue3.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Apr 12, 2024

Deploy Preview for histoire-site ready!

Name Link
🔨 Latest commit 5cd8fd6
🔍 Latest deploy log https://app.netlify.com/sites/histoire-site/deploys/66a0b918405d2e0008dac751
😎 Deploy Preview https://deploy-preview-710--histoire-site.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@juleshry juleshry changed the title remove useNuxtApp mock in nuxt plugin preventing some behaviors (fix #703) fix(plugin-nuxt): remove useNuxtApp mock in nuxt plugin preventing some behaviors (fix #703) Apr 15, 2024
@gjeusel
Copy link

gjeusel commented Apr 27, 2024

Hey there ! I believe there must be a reason why the author decided to entirely mock the useNuxtApp (altough I don't understand it).
Entirely removing the mocked useNuxtApp make histoire fail on my side, not at nuxt build time, but at runtime with a Failed to fetch dynamically imported module from @histoire/plugin-vue.

Maybe a better solution might be to just inline the actual nuxt config in the useNuxtApp composable ?

diff --git a/packages/histoire-plugin-nuxt/runtime/composables.mjs b/packages/histoire-plugin-nuxt/runtime/composables.mjs
deleted file mode 100644
index 9b9a4d8..0000000
--- a/packages/histoire-plugin-nuxt/runtime/composables.mjs
+++ /dev/null
@@ -1 +0,0 @@
-export const useNuxtApp = () => ({ runWithContext: async fn => await fn() })
diff --git a/packages/histoire-plugin-nuxt/src/index.ts b/packages/histoire-plugin-nuxt/src/index.ts
index b284eea..ca3cc8f 100644
--- a/packages/histoire-plugin-nuxt/src/index.ts
+++ b/packages/histoire-plugin-nuxt/src/index.ts
@@ -135,7 +135,17 @@ async function useNuxtViteConfig() {
   }
   const runtimeDir = fileURLToPath(new URL('../runtime', import.meta.url))
   nuxt.options.build.templates.push(
-    { src: join(runtimeDir, 'composables.mjs'), filename: 'histoire/composables.mjs' },
+    {
+      async getContents() {
+        return `
+            export const useNuxtApp = () => ({
+              runWithContext: async (fn) => await fn(),
+              $config: ${JSON.stringify(nuxt.options.runtimeConfig)},
+            })
+        `
+      },
+      filename: "histoire/composables.mjs",
+    },
     { src: join(runtimeDir, 'components.mjs'), filename: 'histoire/components.mjs' },
   )

@juleshry
Copy link
Author

juleshry commented May 3, 2024

Hey @gjeusel,
Thanks for your comment !
It's weird that it doesn't work on your side, maybe our config is not entirely the same.
I think your solution would work fine. But I would like to have some feedback from the team because I don't understand why they want to mock it either.

@0x100101
Copy link

#666 is another related discussion.

@juleshry I implemented your solution and it worked for me on Nuxt 3.10.3 with Histoire 0.17.15

@gjeusel I think you're right to point out that there must be a good reason it was stubbed. But just copying the config in to the stub doesn't solve the problem completely either. (See #666)

Until we completely understand the intention behind the stub it's hard to propose a fix.

@Akryum
Copy link
Member

Akryum commented Jun 25, 2024

I wonder why the tests don't run on this PR 🤔

@awacode21
Copy link

anything new? i try to render https://nuxt.com/modules/icon in histoire but it is not working

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants