natemoo-re / astro-icon

Inline and sprite-based SVGs in Astro made easy!
https://astroicon.dev
Other
990 stars 57 forks source link

Installed icon packs are not detected in a monorepo setup #187

Open lukasgreussing opened 5 months ago

lukasgreussing commented 5 months ago

What version of astro-icon are you using?

v1.0.2

Astro Info

Astro                    v4.1.1
Node                     v20.10.0
System                   Windows (x64)
Package Manager          npm
Output                   static
Adapter                  none
Integrations             none

If this issue only occurs in one browser, which browser is a problem?

No response

Describe the Bug

We've encountered an issue where icon packs are not being detected in a monorepo setup, particularly when there's only a single package.json in the root directory. It appears that the issue may be connected to the changes made in (#175), which might have altered the way icon packs are detected in specific project setups.

Screenshot 2024-01-06 144041

Unfortunately the minimal reproduction example is not working, becauce I'm unable to install the platform dependencies on stackblitz required by nx but the monorepo structure should still be recognizable.

What's the expected result?

Installed icon packs should be detected correctly in a monorepo setup, regardless of the location of the package.json file.

Link to Minimal Reproducible Example

https://stackblitz.com/edit/withastro-astro-q4a1bw?file=apps%2Fplanetary-proxima%2Fsrc%2Fpages%2Findex.astro&view=editor

natemoo-re commented 5 months ago

This is totally valid! I think we'll have to use something like escalade to find the actual root of the project rather than assuming package.json exists in the current directory.

robin-alphasophia commented 4 months ago

Has anything happened here? We have the same problem - it prevents us from upgrading to the new astro and astro-image version.

Alterantively, if solving this is too hard / time consuming, would it make sense to just add a config option to the icon() function, so that we can point to the folder (does not have to be found automatically, can be set manually once).

luchillo17 commented 3 months ago

I've found this issue is very common in Astro projects, even if I define the root I still have to modify the directories of many integrations with an absolute path based on current file:

// astro.config.mjs -> as it is mjs we use `import.meta.dirname` instead of `__dirname`.
import tailwind from '@astrojs/tailwind';
import vue from '@astrojs/vue';
import icon from 'astro-icon';
import { defineConfig } from 'astro/config';
import { join } from 'node:path';

// https://astro.build/config
export default defineConfig({
  root: import.meta.dirname,
  outDir: '../../dist/apps/portfolio',
  integrations: [
    tailwind({
      configFile: join(import.meta.dirname, './tailwind.config.mjs'),
    }),
    icon({
      iconDir: join(import.meta.dirname, './src/assets/icons'),
    }),
    vue(),
  ],
  i18n: {
    defaultLocale: 'en',
    locales: ['en', 'es'],
  },
});
mwood23 commented 3 months ago

Patch for anyone that's interested!

diff --git a/node_modules/astro-icon/dist/loaders/loadIconifyCollections.js b/node_modules/astro-icon/dist/loaders/loadIconifyCollections.js
index dc48df0..060ca03 100644
--- a/node_modules/astro-icon/dist/loaders/loadIconifyCollections.js
+++ b/node_modules/astro-icon/dist/loaders/loadIconifyCollections.js
@@ -45,10 +45,17 @@ export async function loadCollection(name, autoInstall) {
 async function detectInstalledCollections(root) {
     try {
         let packages = [];
-        const text = await readFile(new URL("./package.json", root), {
+        /**
+         * This doesn't recurse up directories to make it work for monorepos that have
+         * all dependencies at the root package.json.
+         *
+         * https://github.com/natemoo-re/astro-icon/issues/187
+         */
+        const text = await readFile(new URL("../../package.json", root), {
             encoding: "utf8",
         });
         const { dependencies = {}, devDependencies = {} } = JSON.parse(text);
+
         packages.push(...Object.keys(dependencies));
         packages.push(...Object.keys(devDependencies));
         const collections = packages
luchillo17 commented 3 months ago

@mwood23 Wouldn't that patch force everyone to use the monorepo folder structure since the location of the package.json would be hardcoded to be 2 folder levels up from the root config file?

mwood23 commented 3 months ago

It's just a patch that folks can toss into their repo to unblock using a tool like patch-package. It's not the resolution to the bug.