P4sca1 / nuxt-headlessui

Headless UI integration for Nuxt. Completely unstyled, fully accessible UI components, designed to integrate beautifully with Tailwind CSS.
MIT License
166 stars 7 forks source link

Small improvement suggestions #6

Closed pi0 closed 1 year ago

pi0 commented 1 year ago

Hi! Following up https://github.com/nuxt/modules/pull/476, Really nice and minimal module love it! There is only one small issue i could find while doing review. Since @headlessui/vue is a _nested dependency of your module, when calling resolvePath you might need to set second argument paths that includes node_modules/@headlessui actually. Nuxt will by default search in user node_modules which is assumption that users directly added @headlessui/vue to their dependencies as a peer dependency.

P4sca1 commented 1 year ago

Hey! Thank you for your review :)

Does this only apply to package managers like PNPM or Yarn with PNP enabled, that don't add any dependency (including their dependencies) to the top level node_modules folder?

The second argument for resolvePath is an object. Where exactly can I add an additional search path? In the end I want the following directory: @headlessui/vue/dist/components/. Therefore I need to search in ${nuxtRootDir}/node_modules and ${nuxtRootDir}/node_modules/nuxt-headlessui/node_modules. Is that correct? Do I need to use resolvePath in this context or is a simple join sufficient?

pi0 commented 1 year ago

Does this only apply to package managers like PNPM or Yarn with PNP enabled, that don't add any dependency (including their dependencies) to the top level node_modules folder?

Exactly. But generally is also a good practice to avoid depending on implicit parent node_modules structure. This way we pick the right version of package even if hoisted

The second argument for resolvePath is an object. Where exactly can I add an additional search path?

Adding import.meta.url should be enough (it search up from dist/index.mjs to ../node_modules). Please let me know if this didn't work i can check 👍🏼

P4sca1 commented 1 year ago

:tada: This issue has been resolved in version 1.0.2 :tada:

The release is available on:

Your semantic-release bot :package::rocket: