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(nuxt): check has pinia module #359

Merged
merged 1 commit into from
Dec 16, 2024
Merged

Conversation

dnldsht
Copy link
Contributor

@dnldsht dnldsht commented Nov 22, 2024

Description

This PR fixes an issue that occurs when a Nuxt module adds another module internally.

For example, if you add the following in a Nuxt module:

installModule('@pinia/nuxt')
installModule('pinia-plugin-persistedstate/nuxt')

It triggers the following error:

The `@pinia/nuxt` module was not found, `pinia-plugin-persistedstate/nuxt` will not work.

This issue arises due to the way the `hasNuxtModule function operates:

function hasNuxtModule(moduleName, nuxt = useNuxt()) {
  return nuxt.options._installedModules.some(({ meta }) => meta.name === moduleName) || // check modules to be installed
  nuxt.options.modules.some((m) => moduleName === resolveNuxtModuleEntryName(m))
}

The meta name of @pinia/nuxt is always pinia in _installedModules. @pinia/nuxt is only present in nuxt.options.modules when declared in the nuxt.config

The root of the problem lies in the meta.name property. For @pinia/nuxt, its meta.name is always pinia in nuxt.options._installedModules. However, @pinia/nuxt is only present in nuxt.options.modules when explicitly declared in nuxt.config.

To reproduce the issue, check this Stackblitz example:

https://stackblitz.com/edit/github-ymapkt?file=src%2Fmodule.ts

@dnldsht dnldsht requested a review from prazdevs as a code owner November 22, 2024 15:08
prazdevs
prazdevs previously approved these changes Dec 7, 2024
@prazdevs prazdevs self-requested a review December 7, 2024 17:44
@prazdevs prazdevs dismissed their stale review December 7, 2024 17:44

re-review

Copy link
Owner

@prazdevs prazdevs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alright checked for most cases, and it seems to work better this way ! 👍

@prazdevs prazdevs merged commit 0b201b2 into prazdevs:main Dec 16, 2024
1 check passed
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.

2 participants